Skip to content
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

Switch to case insensitive comparison of HTTP headers #534

Closed
wants to merge 1 commit into from

Conversation

jonsten
Copy link

@jonsten jonsten commented Jul 13, 2021

Fixes #533

A bug was introduced in 67cefb2 where http headers no longer was compared case insensitively. This causes problems for Artifactory instances that run behind proxys that normalizes the http headers.

This change fixes the issue by mimicking the implementation in org.apache.http.message.HeaderGroup by using String#equalsIgnoreCase(). Which is the old and correct behaviour.

I haven't added any tests nor tried to build the code base since Gradle is unable to find some dependencies :(

  • All tests passed. If this feature is not already covered by the tests, I added new tests.

A bug was introduced in 67cefb2 where http headers no longer was
compared case insensitively. This causes problems for Artifactory
instances that run behind proxys that normalizes the http headers.

This change fixes the issue by mimicking the implementation in
org.apache.http.message.HeaderGroup by using String#equalsIgnoreCase().
Which is the old and correct behaviour.
@github-actions
Copy link

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


Jon Sten seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@jonsten
Copy link
Author

jonsten commented Jul 13, 2021

Looks like we don't have any CLA signed. It will probably take a few weeks/months to get that setup through legal considering that it is vacation time right now...

@yahavi
Copy link
Member

yahavi commented Jul 18, 2021

@jonsten,
Thanks for your contribution!
Would you like that we will create a separate PR to fix this issue?

@jonsten
Copy link
Author

jonsten commented Jul 18, 2021

That is fine with me, thanks!

@yahavi
Copy link
Member

yahavi commented Jul 18, 2021

Thanks, @jonsten. I created #539 to fix this issue.

@yahavi yahavi closed this Jul 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DependenciesDownloadHelper#downloadArtifactMetaData() handles header case incorrectly
2 participants