[kamailio/kamailio] http_client: Fixed bug, so user-agent header now is added to HTTP request (#674)
16 Jun
2016
16 Jun
'16
4:09 p.m.
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
17 Jun
17 Jun
12:19 p.m.
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
12:30 p.m.
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
12:35 p.m.
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
12:35 p.m.
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
12:37 p.m.
@@ -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
12:39 p.m.
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
12:43 p.m.
@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
12:50 p.m.
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
4 Jul
4 Jul
4:24 p.m.
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
4:24 p.m.
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
3093
days inactive
3111
days old
10 comments
3 participants
participants (3)
-
Daniel-Constantin Mierla -
Jurijs Ivolga
-
Olle E. Johansson