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

utils: introduced the ability to add User-Agent header in HTTP requests. #670

Closed
wants to merge 1 commit into from

Conversation

os11k
Copy link
Contributor

@os11k os11k commented Jun 14, 2016

following commit will introduce possibility to add User-Agent header
for http_query by adding utils module parameter - user_agent,
if none is defined then no User-Agent header is added.

following commit will introduce possibility to add User-Agent header
for http_query by adding utils module parameter - user_agent,
if none is defined then no User-Agent header is added.
@sipidronov
Copy link
Contributor

sipidronov commented Jun 14, 2016

Passing str when char* expected doesn't look correct for me.

@os11k
Copy link
Contributor Author

os11k commented Jun 14, 2016

I'm very sorry, this is my first commit, so I might ask not so very clever questions. :)
I tried to use same logic for user_agent as for xcap_table, which is string.

@miconda
Copy link
Member

miconda commented Jun 14, 2016

@sipidronov good catch, indeed. It should be user_agent.s and only set if the user_agent.len>0. Even in the current form works because .s is the firs field in the str structure, that is not safe for future.

I also think the variable and parameter should be prefixed with http_, as those related to the http query of utils are (e.g., timeout), to be more clear that it refers to the http functionality from the module.

@oej
Copy link
Member

oej commented Jun 14, 2016

This already exists in the http_client module. We're planning to remove http from utils, so why extend it with something you can already get in http_client?

@sipidronov
Copy link
Contributor

@miconda or use PARAM_STRING from the very beginning. BTW looks like passing NULL to API call is ok for curl.

@os11k
Copy link
Contributor Author

os11k commented Jun 14, 2016

Historically we are using this module and we need possibility to add user-agent header for HTTP requests, if you plan to remove this module and you think that nobody needs such functionality, then probably we can drop this pull request. Thank you!

@oej
Copy link
Member

oej commented Jun 14, 2016

The functionality will only be added to coming releases and the plan is to delete the function from the utils module in favour of the new modules. The functionality already exist in 4.4 http_client module. Closing pull request.

@oej oej closed this Jun 14, 2016
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.

None yet

4 participants