[kamailio/kamailio] http_client: Fixed bug, so user-agent header now is added to HTTP request (#674)
This commit add minor fix, so user-agent header now is added for HTTP requests.
I'm not a C programmer, but this fix makes it work. I highly recommend to review this fix. :) You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/674
-- Commit Summary --
* http_client: Fixed user-agent
-- File Changes --
M modules/http_client/functions.c (3)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/674.patch https://github.com/kamailio/kamailio/pull/674.diff
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674
as_asciiz() allocs pkg memory, so if it is not only one at startup, but every time for each http query, then this commit is introducing a pkg meamory leak.
Note that the modparams are zero-terminated strings, even if they are stored in str struct, so the .s field can be used for functions expecting zero-terminated strings.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674#issuecomment-226721826
After clarification from Olle it looks like that this pull request should be closed, cause he never intended to add header User-Agent with http_client_query function. Meanwhile it looks like there are still bug, cause http_connect function do not adds User-Agent header anyway.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674#issuecomment-226724221
No discussion here on this tracker item. Was it on mailing lists?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674#issuecomment-226725289
The bug with as_asciiz may be mine, I need to check that throughout the code. Thanks for the feedback!
I run http_connect with user defined headers in my tests. Will check if something broke that part. Thanks Jurij for the feedback.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674#issuecomment-226725345
@@ -56,6 +56,7 @@ typedef struct { char *cacert; char *ciphersuites; char *http_proxy; + char *useragent; unsigned int authmethod; unsigned int http_proxy_port; unsigned int tlsversion; @@ -206,6 +207,7 @@ static int curL_query_url(struct sip_msg* _m, const char* _url, str* _dst, const res |= curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_function); res |= curl_easy_setopt(curl, CURLOPT_WRITEDATA, &stream);
+ res |= curl_easy_setopt(curl, CURLOPT_USERAGENT, params->useragent);
if (res != CURLE_OK) { /* PANIC */ @@ -379,6 +381,7 @@ int curl_con_query_url(struct sip_msg* _m, const str *connection, const str* url query_params.cacert = default_tls_cacert; query_params.ciphersuites = conn->ciphersuites; query_params.tlsversion = conn->tlsversion; + query_params.useragent = conn->useragent; query_params.verify_peer = conn->verify_peer; query_params.verify_host = conn->verify_host; query_params.timeout = conn->timeout;
This code make http_connect to send user_agent. Not sure if this is correct. :)
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674#issuecomment-226725634
Daniel: The http_client_query was meant to duplicate existing behaviour, including limitations. All new features only go to the http_connect part. Adding all features to the old function doesn't make sense to me. I will document that better.
Jurij: Thanks, will check the code. Something must have gone wrong in my merges.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674#issuecomment-226726047
@os11k - see https://guides.github.com/features/mastering-markdown/ (link is also below the comments form) in order to see the Markdown format that you can use to properly wrap patches and code in the comments.
@oej - I think that the http_query name is more suggestive and needs to be preserved if we remove from the utils module and see no reason not to enhance it. But it can be made just an alias to http_connect() function if it has the same parameters, whenever it is removed from utils.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674#issuecomment-226727101
http_connect has different parameters as it uses the connection configuration paradigm. http_query is just saved for backwards compatibility, nothing else. I agree on preservation, but not maintaining it or adding new stuff to it. Keep it as it is, focus on the new code that is far ahead from http_query in features.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674#issuecomment-226728423
Closing this one -- I actually committed a fixed version of this patch -- it was useful recently when migrating a config, as the new config function interface makes it a more complex migration process from utils module.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674#issuecomment-230289890
Closed #674.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/674#event-712033846
-
Daniel-Constantin Mierla -
Jurijs Ivolga
-
Olle E. Johansson