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

httpclient: Add ability to pass custom headers #1251

Merged
merged 1 commit into from
Jul 15, 2019

Conversation

doanac
Copy link
Collaborator

@doanac doanac commented Jul 8, 2019

This allows custom users of libaktualizr to pass custom headers
with HTTP requests sent to the server.

Signed-off-by: Andy Doan [email protected]

@doanac
Copy link
Collaborator Author

doanac commented Jul 8, 2019

Feel free to reject this PR. My team has a small set of out-of-tree patches we have that would never be of interest to you guys. I thought this one might be innocent enough to try and get upstream.

@pattivacek
Copy link
Collaborator

This seems harmless, and I'm open to it. @lbonn and @eu-smirnov any opinions?

@lbonn
Copy link
Contributor

lbonn commented Jul 11, 2019

Yes, we can probably merge it

@@ -46,7 +46,8 @@ static size_t writeString(void* contents, size_t size, size_t nmemb, void* userp
return size * nmemb;
}

HttpClient::HttpClient() : user_agent(std::string("Aktualizr/") + aktualizr_version()) {
HttpClient::HttpClient(std::vector<std::string>* extra_headers)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be **const** std::vector<std::string>*, just minor comment.

@mike-sul
Copy link
Collaborator

It harmless as long as an user knows what he/she/it does :), there are number of headers that can lead to not desired response, e.g. Last-Modified / If-Modified-Since / E-Tag, etc.

@doanac
Copy link
Collaborator Author

doanac commented Jul 11, 2019

I made the argument const as noted by mike-sul. Again, no big deal, but it helps me manage my out-of-tree patch queue a little easier.

@pattivacek
Copy link
Collaborator

Would you mind adding a small unit test to src/libaktualizr/http/httpclient_test.cc or somewhere else appropriate? Otherwise I fear that this could easily accidentally break or unintentionally get deleted without anyone but you noticing.

This allows custom users of libaktualizr to pass custom headers
with HTTP requests sent to the server.

Signed-off-by: Andy Doan <[email protected]>
@doanac
Copy link
Collaborator Author

doanac commented Jul 13, 2019

Good idea on the test. I've added one that takes advantage of the fact the fake server has a test the checks the Authorization header.

@lbonn lbonn merged commit 46b2884 into advancedtelematic:master Jul 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants