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

transports: http: fix custom headers not being applied #5387

Merged
merged 1 commit into from Feb 7, 2020

Conversation

pks-t
Copy link
Member

@pks-t pks-t commented Feb 6, 2020

In commit b9c5b15 (http: use the new httpclient, 2019-12-22), the HTTP
code got refactored to extract a generic HTTP client that operates
independently of the Git protocol. Part of refactoring was the creation
of a new git_http_request struct that encapsulates the generation of
requests. Our Git-specific HTTP transport was converted to use that in
generate_request, but during the process we forgot to set up custom
headers for the git_http_request and as a result we do not send out
these headers anymore.

Fix the issue by correctly setting up the request's custom headers.


Fixes #5385.

@ethomson
Copy link
Member

ethomson commented Feb 6, 2020

Oops, what an embarassing oversight. Thanks @pks-t, this looks good to me.

Maybe we should have a network test that adds an Authorization header so that we verify that we don't break this in the future. (We can follow up with that later though to get this merged quickly.)

@pks-t
Copy link
Member Author

pks-t commented Feb 6, 2020

Maybe we should have a network test that adds an Authorization header so that we verify that we don't break this in the future. (We can follow up with that later though to get this merged quickly.)

I'd definitely love to have a test. Let me see whether I can add one in online::clone using Basic auth.

@pks-t pks-t force-pushed the pks/transport-http-custom-headers branch from c6b04fc to 80b683f Compare February 6, 2020 11:28
@pks-t
Copy link
Member Author

pks-t commented Feb 6, 2020

Amended a test. The test works for me if setting up my own creds and fails without the fix.

@pks-t pks-t force-pushed the pks/transport-http-custom-headers branch 2 times, most recently from 16dc76c to 46228d8 Compare February 7, 2020 10:13
@pks-t
Copy link
Member Author

pks-t commented Feb 7, 2020

Modified the test to not require any environment variables being set, as that caused CI to skip it.

In commit b9c5b15 (http: use the new httpclient, 2019-12-22), the HTTP
code got refactored to extract a generic HTTP client that operates
independently of the Git protocol. Part of refactoring was the creation
of a new `git_http_request` struct that encapsulates the generation of
requests. Our Git-specific HTTP transport was converted to use that in
`generate_request`, but during the process we forgot to set up custom
headers for the `git_http_request` and as a result we do not send out
these headers anymore.

Fix the issue by correctly setting up the request's custom headers and
add a test to verify we correctly send them.
@pks-t pks-t merged commit 03ac24b into libgit2:master Feb 7, 2020
@pks-t pks-t deleted the pks/transport-http-custom-headers branch February 7, 2020 10:36
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.

recent regression adding http headers to push/fetch
2 participants