Skip to content

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