Skip to content
This repository has been archived by the owner on May 28, 2018. It is now read-only.

Patch for client connection leak when using digest authentication #3546

Closed
wants to merge 5 commits into from

Conversation

agherardi
Copy link
Contributor

No description provided.

@agherardi
Copy link
Contributor Author

@pavelbucek pavelbucek added the OCA label May 15, 2017
@pavelbucek pavelbucek self-assigned this May 15, 2017
@agherardi
Copy link
Contributor Author

Hello - any updates?

@agherardi
Copy link
Contributor Author

I made an additional change. I noticed that when a server returns 2 WWW-Authenticate headers, the first with Basic the second with Digest, the filter picks Basic even though Digest is stronger and therefore Digest should be used per https://tools.ietf.org/html/rfc2617#section-4.6 . I fixed that issue.

@pavelbucek
Copy link
Member

@agherardi you could consider posting the other fix as another PR.

Also, there is no test for the other change. And you are using different formatting that the rest of the code..

@agherardi
Copy link
Contributor Author

agherardi commented May 23, 2017

I'd prefer to keep one PR since both changes are for the same class.

I added unit tests for both changes. i fixed the code formatting issue.

@pavelbucek
Copy link
Member

I won't. Focused changes do help with the debugging in the future.

Not to mention that the commit message doesn't say anything about the other change.

@agherardi agherardi changed the title Patch for client connection leak when using digest authentication Patch for client connection leak when using digest authentication. Choose Digest over Basic if the server returns both May 24, 2017
@agherardi
Copy link
Contributor Author

agherardi commented May 24, 2017

I rolled back the 2nd change. I created a separate pull request for the 2nd change https://github.com/jersey/jersey/pull/3562 .

Other than that, are you OK with changes + tests?

@agherardi agherardi changed the title Patch for client connection leak when using digest authentication. Choose Digest over Basic if the server returns both Patch for client connection leak when using digest authentication May 24, 2017
@agherardi
Copy link
Contributor Author

Any updates? Thanks.

@agherardi
Copy link
Contributor Author

Any updates?

@agherardi
Copy link
Contributor Author

Replaced by https://github.com/jersey/jersey/pull/3751

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants