Skip to content

Fix coverity issues with certificate compression#19440

Closed
tmshort wants to merge 3 commits into
openssl:masterfrom
tmshort:cert-comp-coverity
Closed

Fix coverity issues with certificate compression#19440
tmshort wants to merge 3 commits into
openssl:masterfrom
tmshort:cert-comp-coverity

Conversation

@tmshort

@tmshort tmshort commented Oct 19, 2022

Copy link
Copy Markdown
Contributor
Checklist
  • tests are added or updated

Unless multiple compression algorithms are configured, test 3 is
not run, so anything looking at `test == 3` is considered dead code.
@tmshort tmshort added branch: master Applies to master branch approval: review pending This pull request needs review by a committer approval: otc review pending triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly labels Oct 19, 2022
@tmshort tmshort mentioned this pull request Oct 19, 2022
2 tasks
@tmshort

tmshort commented Oct 19, 2022

Copy link
Copy Markdown
Contributor Author

I don't know how to confirm if this fixes the coverity issues?

@t8m t8m added triaged: bug The issue/pr is/fixes a bug and removed triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly approval: otc review pending labels Oct 19, 2022
@t8m

t8m commented Oct 19, 2022

Copy link
Copy Markdown
Member

I don't know how to confirm if this fixes the coverity issues?

We'll see if they disappear after merging.

@paulidale paulidale added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Oct 19, 2022
@openssl-machine

Copy link
Copy Markdown
Collaborator

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

@paulidale

Copy link
Copy Markdown
Contributor

Merged with end of line white space removal. Thanks.

@paulidale paulidale closed this Oct 20, 2022
openssl-machine pushed a commit that referenced this pull request Oct 20, 2022
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from #19440)
openssl-machine pushed a commit that referenced this pull request Oct 20, 2022
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from #19440)
openssl-machine pushed a commit that referenced this pull request Oct 20, 2022
Unless multiple compression algorithms are configured, test 3 is
not run, so anything looking at `test == 3` is considered dead code.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from #19440)
@tmshort

tmshort commented Oct 24, 2022

Copy link
Copy Markdown
Contributor Author

Where does one see the Coverity results? I looked under actions, but no details are provided.

@t8m

t8m commented Oct 26, 2022

Copy link
Copy Markdown
Member

Where does one see the Coverity results? I looked under actions, but no details are provided.

https://scan.coverity.com/projects/openssl

@t8m

t8m commented Oct 26, 2022

Copy link
Copy Markdown
Member

The 1516093 is still unfixed by this PR. IMO there should be some maximum possible length check on uclen.

@tmshort

tmshort commented Oct 26, 2022

Copy link
Copy Markdown
Contributor Author

The 1516093 is still unfixed by this PR. IMO there should be some maximum possible length check on uclen.

That might be the case. Coverity isn't always clear as to how to fix things.

beldmit pushed a commit to beldmit/openssl that referenced this pull request Dec 26, 2022
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from openssl#19440)
beldmit pushed a commit to beldmit/openssl that referenced this pull request Dec 26, 2022
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from openssl#19440)
beldmit pushed a commit to beldmit/openssl that referenced this pull request Dec 26, 2022
Unless multiple compression algorithms are configured, test 3 is
not run, so anything looking at `test == 3` is considered dead code.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
(Merged from openssl#19440)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants