Fixed connection leak in basic and digest authenticators if 401 response has content#3859
Fixed connection leak in basic and digest authenticators if 401 response has content#3859agherardi wants to merge 9 commits into
Conversation
| return bytesToHex(bytes); | ||
| } | ||
|
|
||
| private static void discardInputAndClose(InputStream is) { |
There was a problem hiding this comment.
Do we really need to consume the stream before it is closed?
There was a problem hiding this comment.
actually yes.. because otherwise this leads to leak or deadlock...
There was a problem hiding this comment.
Consuming the stream makes it possible for the connection to be reused (unless the server closes the connection, of course).
| } | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
This discardInputAndClose is once again in HttpAuthenticationFilter in PR3860. Can you put it to the AuthenticationUtil as discussed in PR 3862?
68962a1 to
7cbc9bb
Compare
…nse has content Signed-off-by: agherardi <alessandro.gherardi@yahoo.com>
Signed-off-by: agherardi <alessandro.gherardi@yahoo.com>
Signed-off-by: agherardi <alessandro.gherardi@yahoo.com>
|
Hi - Any updates? |
|
There is class AuthenticationUtil #discardInputAndClose already from the previous PR. Can you please rebase and resolve conflicts? |
|
Conflict resolved. There is a test error. Looking into it. |
|
The test issue is caused by a missing "@get" in front of this https://github.com/eclipse-ee4j/jersey/blob/master/connectors/apache-connector/src/test/java/org/glassfish/jersey/apache/connector/AuthTest.java#L157 test method, due to a merge conflict with https://github.com/eclipse-ee4j/jersey/pull/3858/files. I re-added the "@get" with this merge request. Otherwise, please feel free to back out #3858, I'll re-submit with the fixed test. Or you can approve #3875 . |
|
Thanks. This merge request should be ready to be merged now. |
Signed-off-by: agherardi <alessandro.gherardi@yahoo.com>
|
The IP validation check is failing. Apparently, a couple of merge commits don't have either the author or the signed-off line in the commit message. I'm assuming the changes can still be merged since the "important" commit has both author and signed-off. If that's not the case, please let me know - I'll open a new pull request. Thanks. |
|
There seems to be some discrepancy in the sign-off/commit users alessandro.gherardi[at]yahoo.com vs alessandro.gherardi[at]schneider-electric.com. Not sure where it took the latter user. Can you please better create a new PR? Thanks |
|
Changed re-submitted as #3878 . Closing this request. |
4 participants
This is a clone of https://github.com/jersey/jersey/pull/3623