http_client: Fixed bug, so user-agent header now is added to HTTP request

[os11k]

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. :)

[miconda]

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.

[os11k]

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.

[miconda]

No discussion here on this tracker item. Was it on mailing lists?

[oej]

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.

[os11k]

@@ -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. :)

[oej]

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.

[miconda]

@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.

[oej]

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.

[miconda]

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.