Skip to content

Switch to case insensitive comparison of HTTP headers#534

Closed
jonsten wants to merge 1 commit intojfrog:masterfrom
jonsten:httpHeadersFix
Closed

Switch to case insensitive comparison of HTTP headers#534
jonsten wants to merge 1 commit intojfrog:masterfrom
jonsten:httpHeadersFix

Conversation

@jonsten
Copy link
Copy Markdown

@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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Author

jonsten commented Jul 18, 2021

That is fine with me, thanks!

@yahavi
Copy link
Copy Markdown
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