Re[2]: [PATCH] add support for http+unix protocol

[Konstantin Belyavskiy]

src/httpc.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/src/httpc.c b/src/httpc.c
index 84ea67aba..885ce1b98 100644
--- a/src/httpc.c
+++ b/src/httpc.c
@@ -32,10 +32,36 @@
#include "httpc.h"

#include <assert.h>
+#include <ctype.h>
#include <curl/curl.h>

#include "fiber.h"

+static const char *HTTP_LOCALHOST = "http://localhost";
+static const char *HTTP_UNIX_PREFIX = "http+unix://";
+
+/**
+ * Function to decode encoded symbols in http url.
+ */
+static int
+urldecode(const char *s, const char *end, char *dec)
+{
+ char *o;
+ int c;
+
+ for (o = dec; s != end; o++) {
+ c = *s++;
+ if (c == '+') c = ' ';
+ else if (c == '%' && (!isxdigit(*s++) ||
+ !isxdigit(*s++) ||
+ !sscanf(s - 2, "%2x", &c)))
+ return -1;
+ *o = c;
+ }
+ *o = '\0';
+ return o - dec;
+}
+
/**
* libcurl callback for CURLOPT_WRITEFUNCTION
* @see https://curl.haxx.se/libcurl/c/CURLOPT_WRITEFUNCTION.html
@@ -134,7 +160,32 @@ httpc_request_new(struct httpc_env *env, const char *method,
curl_easy_setopt(req->curl_request.easy, CURLOPT_CUSTOMREQUEST, method);
}

- curl_easy_setopt(req->curl_request.easy, CURLOPT_URL, url);
+ const int prefix_len = strlen(HTTP_UNIX_PREFIX);
+ if (strncmp(url, HTTP_UNIX_PREFIX, prefix_len) == 0) {
+ const int url_len = strlen(url);
+ struct region region;
+ struct slab_cache cache;
+ region_create(&region, &cache);
+ char *http_url = (char *) region_alloc(&region,
+ strlen(HTTP_LOCALHOST) + url_len - prefix_len + 1);
+
+ const char *socket_path_start = url + prefix_len;
+ const char *socket_path_end = strchr(socket_path_start, '/');
+ if (socket_path_end == NULL)
+ socket_path_end = url + url_len;
+
+ char *socket_path = (char *) region_alloc(&region,
+ socket_path_end - socket_path_start + 1);
+ urldecode(socket_path_start, socket_path_end, socket_path);
+ sprintf(http_url, "%s%s", HTTP_LOCALHOST, socket_path_end);
+
+ curl_easy_setopt(req->curl_request.easy, CURLOPT_UNIX_SOCKET_PATH, socket_path);
+ curl_easy_setopt(req->curl_request.easy, CURLOPT_URL, http_url);
+
+ region_free(&region);
+ } else {
+ curl_easy_setopt(req->curl_request.easy, CURLOPT_URL, url);
+ }

curl_easy_setopt(req->curl_request.easy, CURLOPT_FOLLOWLOCATION, 1);
curl_easy_setopt(req->curl_request.easy, CURLOPT_SSL_VERIFYPEER, 1);

 


>Пятница, 26 января 2018, 14:19 +03:00 от Vladimir Davydov <vdavydov.dev at gmail.com>:
>
>Style: Would be nice to add 'httpc: ' to the commit message subject:
>
>  httpc: add support for unix protocol
>
>On Fri, Jan 26, 2018 at 11:51:14AM +0300, Konstantin Belyavskiy wrote:
>> The http+unix:// is http protocol over unix domain sockets.
>> implementation based on original racktear proposal
>> decided to support only http+unix format
>> decided not to use our uri parser
>
>Style: I don't think it's worth enumerating what we decided NOT to do.
>
>> 
>> closes #3040
>
>Style: Should start with a capital letter.
>
>> ---
>> branch: gh-3040-unix-domain-sockets-orig-fix-formatting
>>  src/httpc.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>  src/httpc.h |  1 +
>
>Please add a test.
>
>>  2 files changed, 49 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/httpc.c b/src/httpc.c
>> index 84ea67aba..d5ad1164b 100644
>> --- a/src/httpc.c
>> +++ b/src/httpc.c
>> @@ -36,6 +36,31 @@
>> 
>>  #include "fiber.h"
>> 
>> +static const char *HTTP_LOCALHOST = "http://localhost";
>> +static const char *HTTP_UNIX_PREFIX = "http+unix://";
>> +
>> +/**
>> + * function to decode encoded symbols in http url
>
>Style: Start sentences with a capital letter and end with a dot.
>
>> + */
>> +static int
>> +urldecode(const char *s, const char *end, char *dec)
>> +{
>> +	char *o;
>> +	int c;
>> +
>> +	for (o = dec; s <= end; o++) {
>
>In our code, 'end' usually points to the entry following the last one in
>a set (just like in STL) and this is actually true in your case. So this
>should be:
>
>  for (o = dec; s != end; o++)
>
>> +		c = *s++;
>> +		if (c == '+') c = ' ';
>> +		else if (c == '%' && (!isxdigit(*s++) ||
>> +				      !isxdigit(*s++) ||
>> +				      !sscanf(s - 2, "%2x", &c)))
>> +			return -1;
>> +		*o = c;
>> +	}
>
>I think this function should set the terminating nul.
>
>> +
>> +	return o - dec;
>> +}
>> +
>>  /**
>>   * libcurl callback for CURLOPT_WRITEFUNCTION
>>   * @see  https://curl.haxx.se/libcurl/c/CURLOPT_WRITEFUNCTION.html
>> @@ -134,7 +159,29 @@ httpc_request_new(struct httpc_env *env, const char *method,
>>  		curl_easy_setopt(req->curl_request.easy, CURLOPT_CUSTOMREQUEST, method);
>>  	}
>> 
>> -	curl_easy_setopt(req->curl_request.easy, CURLOPT_URL, url);
>> +	const int prefix_len = strlen(HTTP_UNIX_PREFIX);
>> +	if (strncmp(url, HTTP_UNIX_PREFIX, prefix_len) == 0) {
>> +		const int url_len = strlen(url);
>> +		char *http_url = (char *) malloc(strlen(HTTP_LOCALHOST) +
>> +						 url_len - prefix_len + 1);
>
>Please consider using the region allocator instead of malloc if
>possible - it is much more efficient in terms of performance.
>And don't forget to handle allocation failures.
>
>> +
>> +		const char *socket_path_start = url + prefix_len;
>> +		const char *socket_path_end = strchr(socket_path_start, '/');
>> +		if (socket_path_end == NULL)
>> +			socket_path_end = url + url_len;
>> +
>> +		char *socket_path = (char *) malloc(socket_path_end - socket_path_start + 1);
>> +		urldecode(socket_path_start, socket_path_end, socket_path);
>> +		sprintf(http_url, "http://localhost%s", socket_path_end);
>
>Please reuse HTTP_LOCALHOST here:
>
>  sprintf(http_url, HTTP_LOCALHOST "%s", socket_path_end);
>
>> +
>> +		curl_easy_setopt(req->curl_request.easy, CURLOPT_UNIX_SOCKET_PATH, socket_path);
>> +		curl_easy_setopt(req->curl_request.easy, CURLOPT_URL, http_url);
>> +
>> +		free(http_url);
>> +		free(socket_path);
>> +	} else {
>> +		curl_easy_setopt(req->curl_request.easy, CURLOPT_URL, url);
>> +	}
>> 
>>  	curl_easy_setopt(req->curl_request.easy, CURLOPT_FOLLOWLOCATION, 1);
>>  	curl_easy_setopt(req->curl_request.easy, CURLOPT_SSL_VERIFYPEER, 1);
>> diff --git a/src/httpc.h b/src/httpc.h
>> index 3358fd914..8c3e75f75 100644
>> --- a/src/httpc.h
>> +++ b/src/httpc.h
>> @@ -34,6 +34,7 @@
>>  #include <small/region.h>
>>  #include <small/mempool.h>
>>  #include <tarantool_ev.h>
>> +#include <ctype.h>
>
>Please avoid including in headers - this slows down compilation.
>Include in C files if possible.


С уважением,
Konstantin Belyavskiy
k.belyavskiy at tarantool.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20180126/7925f461/attachment.html>