-
Notifications
You must be signed in to change notification settings - Fork 207
Bugfix/362 #374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bugfix/362 #374
Conversation
@debugthings Is there any way you can separate the bug fix for #369 into its own PR? I'm getting requests from external users regarding the status of closing that specific CVE but I'm not knowledgeable enough about this SDK to review all of what you've included here. |
compile group: 'org.apache.commons', name: 'commons-lang3', version: '3.1' | ||
compile group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.3.5' | ||
compile group: 'com.google.guava', name: 'guava', version: '12.0.1' | ||
compile ([group: 'eu.infomas', name: 'annotation-detector', version: '3.0.4']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it mean that our customer who used core and web with transitive dependencies and relied on them will need to copy them manually from now on? If so, you'd need to update a documentation in Azure Docs repo as well and PR them :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will opt to keep this transitive and relocate the javax packages out of the root of the JAR. This should satisfy that requirement. Look for an updated commit soon
core/build.gradle
Outdated
compile ([group: 'eu.infomas', name: 'annotation-detector', version: '3.0.4']) | ||
compile ([group: 'commons-io', name: 'commons-io', version: '2.4' ]) | ||
compile ([group: 'org.apache.commons', name: 'commons-lang3', version: '3.1']) | ||
compile ([group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.5.3']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember there were some version specific things in our code around apache.http - did you check that code and telemetry transmission are fine with that change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version does work. But per @sharwell I will move this to another PR to be reviewed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@debugthings Thank you so much for this. I know it's more work for you but I know it means a lot to one downstream user.
Based on feed back from @isaac84 and PR #367 I've updated the core JAR to remove the transitive dependencies for guava, which depends on JSR305 which has it's own dependency on javax.annotation.
I also removed the transitive dependency from core and agent, even though it's not really required in this situation as the web JAR could have a need for a shadow jar in the future. This will save on bug hunts later.
Lastly I updated to the latest version of the httpclient based on the recommendation of CVE-2015-5262 however this vulnerability is classified as a DoS. In short it is a bug in the httpclient that does not honor the SSL handshake timeout.