You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2020/10/29 16:43:12 UTC
[GitHub] [httpcomponents-client] artem-smotrakov opened a new pull request #262: Fixed and suppressed several findings from LGTM.com
artem-smotrakov opened a new pull request #262:
URL: https://github.com/apache/httpcomponents-client/pull/262
LGTM.com offers static analysis for open-source projects. It's based on CodeQL engine. The current report for Apache HttpClient is pretty good. There are only 19 findings. I had a look at them, and didn't find anything severe. Nevertheless, I'd like to propose fixing some minor findings and suppressing the rest. Suppressing the warnings would make it simpler to triage new issues in the future.
Here is a list of updates:
- Fixed a few possible null dereferences
- Fixed a few possible out-of-bound array ops
- Suppressed warnings for weak cryptographic algorithms for NTLM and Digest authentication schemes
- Suppressed a few false-positives for null dereference
- Suppressed a few warnings for overriding a synchronized method without synchronization
- Added a couple of test cases
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] garydgregory commented on pull request #262: Fixed and suppressed several findings from LGTM.com
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #262:
URL: https://github.com/apache/httpcomponents-client/pull/262#issuecomment-719043584
> @garydgregory I agree with the annotation, but would analyze the rest for validity.
@artem-smotrakov please update the PR without vendor specific comments and annotations.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] garydgregory commented on pull request #262: Fixed and suppressed several findings from LGTM.com
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #262:
URL: https://github.com/apache/httpcomponents-client/pull/262#issuecomment-718888496
Personally -1, I'm not a fan of adding non-standard, some .com's proprietary annotations and source comments to our sources. I would be more open if this type of annotations were in another file just one does for FindBugs, SpotBugs, Checkstyle, Apache RAT.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] artem-smotrakov commented on pull request #262: Fixed and suppressed several findings from LGTM.com
Posted by GitBox <gi...@apache.org>.
artem-smotrakov commented on pull request #262:
URL: https://github.com/apache/httpcomponents-client/pull/262#issuecomment-718916257
> Personally -1, I'm not a fan of adding non-standard, some .com's proprietary annotations and source comments to our sources. I would be more open if this type of annotations were in another file just one does for FindBugs, SpotBugs, Checkstyle, Apache RAT.
Yup, I don't like it either. Unfortunately, it is not possible to add a file with suppressions
https://lgtm.com/help/lgtm/alert-suppression#:~:text=LGTM%20generates%20alerts%20when%20it,of%20making%20the%20code%20better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on pull request #262: Fixed and suppressed several findings from LGTM.com
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #262:
URL: https://github.com/apache/httpcomponents-client/pull/262#issuecomment-718953001
@artem-smotrakov I agree with @garydgregory. We will happily accept the fixes but without vendor specific annotations.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] michael-o commented on pull request #262: Fixed and suppressed several findings from LGTM.com
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #262:
URL: https://github.com/apache/httpcomponents-client/pull/262#issuecomment-718905584
@garydgregory I agree with the annotation, but would analyze the rest for validity.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] carterkozak commented on a change in pull request #262: Fixed and suppressed several findings from LGTM.com
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #262:
URL: https://github.com/apache/httpcomponents-client/pull/262#discussion_r514443722
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/HttpRFC7578Multipart.java
##########
@@ -137,13 +137,12 @@ protected void formatMultipartHeader(final MultipartPart part, final OutputStrea
for (int i = 0; i < bytes.length; i++) {
final int b = bytes[i];
if (b == ESCAPE_CHAR) {
- try {
- final int u = digit16(bytes[++i]);
- final int l = digit16(bytes[++i]);
- buffer.append((char) ((u << 4) + l));
- } catch (final ArrayIndexOutOfBoundsException e) {
- throw new DecoderException("Invalid URL encoding: ", e);
+ if (i >= bytes.length - 2) {
+ throw new DecoderException("Invalid URL encoding: too short");
}
+ final int u = digit16(bytes[++i]);
+ final int l = digit16(bytes[++i]);
+ buffer.append((char) ((u << 4) + l));
Review comment:
+1 for validating preconditions instead of handling exceptions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org