From a360449bf9d6bc28c8d144172ef5ef20c6a1b49a Mon Sep 17 00:00:00 2001 From: Michael Scott Date: Thu, 9 Nov 2017 14:08:17 -0800 Subject: [PATCH 1/3] net: http: dont add CRLF to protocol In http_request() a CRLF is added to the header information after the protocol is added. 2 CRLF in a row means the header information is done, so following header information will be ignored. Signed-off-by: Michael Scott --- include/net/http_app.h | 2 +- samples/net/http_client/src/main.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/net/http_app.h b/include/net/http_app.h index 7e32e55d5ffe4..af99f2dc5f6bd 100644 --- a/include/net/http_app.h +++ b/include/net/http_app.h @@ -767,7 +767,7 @@ static inline int http_client_send_get_req(struct http_ctx *http_ctx, .method = HTTP_GET, .url = url, .host = host, - .protocol = " " HTTP_PROTOCOL HTTP_CRLF, + .protocol = " " HTTP_PROTOCOL, .header_fields = extra_header_fields, }; diff --git a/samples/net/http_client/src/main.c b/samples/net/http_client/src/main.c index 8b4120c825b1e..7f16defde2818 100644 --- a/samples/net/http_client/src/main.c +++ b/samples/net/http_client/src/main.c @@ -160,7 +160,7 @@ static int do_sync_http_req(struct http_ctx *ctx, req.method = method; req.url = url; - req.protocol = " " HTTP_PROTOCOL HTTP_CRLF; + req.protocol = " " HTTP_PROTOCOL; NET_INFO("[%d] Send %s", count, url); @@ -295,7 +295,7 @@ static int do_async_http_req(struct http_ctx *ctx, req.method = method; req.url = url; - req.protocol = " " HTTP_PROTOCOL HTTP_CRLF; + req.protocol = " " HTTP_PROTOCOL; k_sem_init(&waiter.wait, 0, 1); From 0380bd2f23f5b6a12e442c92a9ee02ef0e79757e Mon Sep 17 00:00:00 2001 From: Michael Scott Date: Fri, 10 Nov 2017 13:20:56 -0800 Subject: [PATCH 2/3] net: http: honor CONFIG_HTTP_CLIENT_NETWORK_TIMEOUT setting We should not use the user suppied timeout setting in http_client_send_req() for the connection timeout. In the previous API the call to tcp_connect() used CONFIG_HTTP_CLIENT_NETWORK_TIMEOUT as the timeout setting. Let's do that here too. This fixes -ETIMEDOUT error generation when using K_NO_WAIT for http_client_send_req(). Signed-off-by: Michael Scott --- subsys/net/lib/http/http_app_client.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/subsys/net/lib/http/http_app_client.c b/subsys/net/lib/http/http_app_client.c index a0598cd3e6f91..a800e8d658de3 100644 --- a/subsys/net/lib/http/http_app_client.c +++ b/subsys/net/lib/http/http_app_client.c @@ -38,6 +38,9 @@ #define HTTP_CONTENT_LEN "Content-Length" #define HTTP_CONT_LEN_SIZE 6 +/* Default network activity timeout in seconds */ +#define HTTP_NETWORK_TIMEOUT K_SECONDS(CONFIG_HTTP_CLIENT_NETWORK_TIMEOUT) + int client_reset(struct http_ctx *ctx) { http_parser_init(&ctx->http.parser, HTTP_RESPONSE); @@ -235,7 +238,7 @@ int http_client_send_req(struct http_ctx *ctx, ctx->http.rsp.cb = cb; - ret = net_app_connect(&ctx->app_ctx, timeout); + ret = net_app_connect(&ctx->app_ctx, HTTP_NETWORK_TIMEOUT); if (ret < 0) { NET_DBG("Cannot connect to server (%d)", ret); return ret; @@ -244,7 +247,7 @@ int http_client_send_req(struct http_ctx *ctx, /* We might wait longer than timeout if the first connection * establishment takes long time (like with HTTPS) */ - if (k_sem_take(&ctx->http.connect_wait, timeout)) { + if (k_sem_take(&ctx->http.connect_wait, HTTP_NETWORK_TIMEOUT)) { NET_DBG("Connection timed out"); ret = -ETIMEDOUT; goto out; From 4e946d24b8e77dcf93aa2363ea2776b75b1b8043 Mon Sep 17 00:00:00 2001 From: Michael Scott Date: Thu, 9 Nov 2017 16:13:04 -0800 Subject: [PATCH 3/3] net: http: client: remove payload send_chunk logic Logic for sending chunks of data is incompatible with adding Content-Length: header. Per https://tools.ietf.org/html/rfc7230#section-3.3.1: "A sender MUST NOT send a Content-Length header field in any message that contains a Transfer-Encoding header field." Going a bit further in my mind: also don't send Transfer-Encoded chunked data either when the Content-Length header is present. In general, there will be problems if the http client library makes payload changes without the user code knowing about it. This patch removes the use of http_send_chunk() from the new HTTP client code and instead sends the payload directly to http_prepare_and_send() This fixes an issue where every available buffer would be allocated with repeating payload data because the for loop in http_request() wasn't ending until we ran out of memory. Signed-off-by: Michael Scott --- subsys/net/lib/http/http_app_client.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/subsys/net/lib/http/http_app_client.c b/subsys/net/lib/http/http_app_client.c index a800e8d658de3..6b37261144ea7 100644 --- a/subsys/net/lib/http/http_app_client.c +++ b/subsys/net/lib/http/http_app_client.c @@ -123,7 +123,6 @@ int http_request(struct http_ctx *ctx, struct http_request *req, s32_t timeout, if (req->payload && req->payload_size) { char content_len_str[HTTP_CONT_LEN_SIZE]; - int i; ret = snprintk(content_len_str, HTTP_CONT_LEN_SIZE, "%u", req->payload_size); @@ -143,17 +142,10 @@ int http_request(struct http_ctx *ctx, struct http_request *req, s32_t timeout, goto out; } - for (i = 0; i < req->payload_size;) { - ret = http_send_chunk(ctx, - req->payload + i, - req->payload_size - i, - user_data); - if (ret < 0) { - NET_ERR("Cannot send data to peer (%d)", ret); - return ret; - } - - i += ret; + ret = http_prepare_and_send(ctx, req->payload, + req->payload_size, user_data); + if (ret < 0) { + goto out; } } else { ret = http_add_header(ctx, HTTP_EOF, user_data);