| Message ID | 20230517084626.104774-1-christian.storm@siemens.com |
|---|---|
| State | Accepted |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | channel_curl: Optionally return body on PUT | expand |
Hi Christian, On 17.05.23 10:46, 'Christian Storm' via swupdate wrote: > PUT may return the sent document applied. This is > currently not returned to channel_curl consumers. > Is it a proprietary solution ? I do not see any reference about body response in RFC7231 (HTTP 1.1). Sure, there are HTTP codes that current implementation does not support. > Signed-off-by: Christian Storm <christian.storm@siemens.com> > --- > corelib/channel_curl.c | 10 ++++-- > suricatta/server_general.c | 3 ++ > suricatta/server_hawkbit.c | 1 + > suricatta/server_lua.c | 70 ++++++++++++++++++-------------------- > 4 files changed, 45 insertions(+), 39 deletions(-) > > diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c > index 1207304e..24a259e3 100644 > --- a/corelib/channel_curl.c > +++ b/corelib/channel_curl.c > @@ -342,6 +342,8 @@ channel_op_res_t channel_map_http_code(channel_t *this, long *http_response_code > case 429: /* Bad Request, i.e., too many requests. Try again later. */ > return CHANNEL_EAGAIN; > case 200: > + case 201: > + case 204: > case 206: > case 226: > return CHANNEL_OK; > @@ -1103,7 +1105,7 @@ static channel_op_res_t channel_put_method(channel_t *this, void *data) > > CURLcode curlrc = curl_easy_perform(channel_curl->handle); > if (curlrc != CURLE_OK) { > - ERROR("Channel put operation failed (%d): '%s'", curlrc, > + ERROR("Channel PUT operation failed (%d): '%s'", curlrc, > curl_easy_strerror(curlrc)); > result = channel_map_curl_error(curlrc); > goto cleanup_header; > @@ -1116,7 +1118,11 @@ static channel_op_res_t channel_put_method(channel_t *this, void *data) > if (channel_data->nocheckanswer) > goto cleanup_header; > > - channel_log_reply(result, channel_data, NULL); > + channel_log_reply(result, channel_data, &outdata); > + > + if (result == CHANNEL_OK) { > + result = parse_reply(channel_data, &outdata); > + } > > cleanup_header: > outdata.memory != NULL ? free(outdata.memory) : (void)0; > diff --git a/suricatta/server_general.c b/suricatta/server_general.c > index 1c8e777e..6f4bce8b 100644 > --- a/suricatta/server_general.c > +++ b/suricatta/server_general.c > @@ -324,6 +324,9 @@ static void *server_progress_thread (void *data) > if (logbuffer) { > channel_data.request_body = logbuffer; > channel_data.method = CHANNEL_PUT; > + /* .format is already specified in channel_data_defaults, > + * but being explicit doesn't hurt. */ > + channel_data.format = CHANNEL_PARSE_NONE; > channel_data.content_type = "application/text"; > result = map_channel_retcode(channel->put(channel, (void *)&channel_data)); > if (result != SERVER_OK) > diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c > index 726ef191..a400c12c 100644 > --- a/suricatta/server_hawkbit.c > +++ b/suricatta/server_hawkbit.c > @@ -1590,6 +1590,7 @@ static server_op_res_t server_send_target_data(void) > } > > channel_data_reply.url = url; > + channel_data_reply.format = CHANNEL_PARSE_NONE; > channel_data_reply.request_body = json_reply_string; > TRACE("URL=%s JSON=%s", channel_data_reply.url, channel_data_reply.request_body); > channel_data_reply.method = CHANNEL_PUT; > diff --git a/suricatta/server_lua.c b/suricatta/server_lua.c > index 579ddc70..bdf34084 100644 > --- a/suricatta/server_lua.c > +++ b/suricatta/server_lua.c > @@ -717,49 +717,45 @@ static int channel_do_operation(lua_State *L, channel_method_t op) > push_result(L, result); > lua_pushnumber(L, result); > lua_newtable(L); > - if (op == CHANNEL_GET || channel_data.method == CHANNEL_POST || > - channel_data.method == CHANNEL_PATCH) { > - push_to_table(L, "http_response_code", channel_data.http_response_code); > - push_to_table(L, "format", channel_data.format); > - #ifdef CONFIG_JSON > - if (channel_data.format == CHANNEL_PARSE_JSON) { > - lua_pushstring(L, "json_reply"); > - if (!channel_data.json_reply || > - !json_to_table(L, channel_data.json_reply)) { > - lua_pushnil(L); > - } > - lua_settable(L, -3); > - > - if (channel_data.json_reply && > - json_object_put(channel_data.json_reply) != 1) { > - ERROR("JSON object should be freed but was not."); > - } > - } > - #endif > - if (channel_data.format == CHANNEL_PARSE_RAW) { > - lua_pushstring(L, "raw_reply"); > - if (!channel_data.raw_reply) { > - lua_pushnil(L); > - } else { > - lua_pushstring(L, channel_data.raw_reply); > - free(channel_data.raw_reply); > - } > - lua_settable(L, -3); > + push_to_table(L, "http_response_code", channel_data.http_response_code); > + push_to_table(L, "format", channel_data.format); > + #ifdef CONFIG_JSON > + if (channel_data.format == CHANNEL_PARSE_JSON) { > + lua_pushstring(L, "json_reply"); > + if (!channel_data.json_reply || > + !json_to_table(L, channel_data.json_reply)) { > + lua_pushnil(L); > } > + lua_settable(L, -3); > > - lua_pushstring(L, "received_headers"); > - lua_newtable(L); > - if (!LIST_EMPTY(channel_data.received_headers)) { > - struct dict_entry *entry; > - LIST_FOREACH(entry, channel_data.received_headers, next) { > - lua_pushstring(L, dict_entry_get_key(entry)); > - lua_pushstring(L, dict_entry_get_value(entry)); > - lua_settable(L, -3); > - } > + if (channel_data.json_reply && > + json_object_put(channel_data.json_reply) != 1) { > + ERROR("JSON object should be freed but was not."); > + } > + } > + #endif > + if (channel_data.format == CHANNEL_PARSE_RAW) { > + lua_pushstring(L, "raw_reply"); > + if (!channel_data.raw_reply) { > + lua_pushnil(L); > + } else { > + lua_pushstring(L, channel_data.raw_reply); > + free(channel_data.raw_reply); > } > lua_settable(L, -3); > } > > + lua_pushstring(L, "received_headers"); > + lua_newtable(L); > + if (!LIST_EMPTY(channel_data.received_headers)) { > + struct dict_entry *entry; > + LIST_FOREACH(entry, channel_data.received_headers, next) { > + lua_pushstring(L, dict_entry_get_key(entry)); > + lua_pushstring(L, dict_entry_get_value(entry)); > + lua_settable(L, -3); > + } > + } > + lua_settable(L, -3); > dict_drop_db(&header_send); > dict_drop_db(&header_receive); > Regards, Stefano
Hi Stefano, > > PUT may return the sent document applied. This is > > currently not returned to channel_curl consumers. > > > > Is it a proprietary solution ? I do not see any reference about body response > in RFC7231 (HTTP 1.1). True. The HTTP standard does not mandate that there is a document returned with a response. In fact, if the HTTP return code (semantically) resembles the body content, it's even wasteful. [This is the reason for defaulting to CHANNEL_PARSE_NONE] However, applications/standards built on top of HTTP add new "rules": For example, https://jsonapi.org/format/#document-top-level reads: "A JSON object MUST be at the root of every JSON:API request and response document containing data." Now, a PUT response may not contain data, so it must not strictly contain (valid) JSON... Anyway, there are others, e.g., jQuery that assumes that if JSON is sent it gets back (valid) JSON qua implementation. Let aside this, it can be considered an optimization for backends that implement this behavior to spare you a GET as you get the GET's (JSON) content in response to your PUT request. This is not proprietary but may also not always be implemented, hence it's an optional feature. So, it's not defined / left unspecified in the HTTP standard, yes, but it's also not violating it. It's not a proprietary SWUpdate extension but an opt-in feature given your backend supports this. It's coming from standards that build on HTTP as mere transport channel. Kind regards, Christian
Hi Christian, On 17.05.23 13:59, 'Christian Storm' via swupdate wrote: > Hi Stefano, > >>> PUT may return the sent document applied. This is >>> currently not returned to channel_curl consumers. >>> >> >> Is it a proprietary solution ? I do not see any reference about body response >> in RFC7231 (HTTP 1.1). > > True. The HTTP standard does not mandate that there is a document > returned with a response. In fact, if the HTTP return code > (semantically) resembles the body content, it's even wasteful. > [This is the reason for defaulting to CHANNEL_PARSE_NONE] > Ok > However, applications/standards built on top of HTTP add new "rules": > For example, https://jsonapi.org/format/#document-top-level reads: > "A JSON object MUST be at the root of every JSON:API request and > response document containing data." > Now, a PUT response may not contain data, so it must not strictly > contain (valid) JSON... > Anyway, there are others, e.g., jQuery that assumes that if JSON is > sent it gets back (valid) JSON qua implementation. > > Let aside this, it can be considered an optimization for backends > that implement this behavior to spare you a GET as you get the GET's > (JSON) content in response to your PUT request. This is not proprietary > but may also not always be implemented, hence it's an optional feature. Ok > > > So, it's not defined / left unspecified in the HTTP standard, yes, but > it's also not violating it. That's the point, yes. Fine. > It's not a proprietary SWUpdate extension > but an opt-in feature given your backend supports this. > It's coming from standards that build on HTTP as mere transport channel. > Best regards, Stefano
diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c index 1207304e..24a259e3 100644 --- a/corelib/channel_curl.c +++ b/corelib/channel_curl.c @@ -342,6 +342,8 @@ channel_op_res_t channel_map_http_code(channel_t *this, long *http_response_code case 429: /* Bad Request, i.e., too many requests. Try again later. */ return CHANNEL_EAGAIN; case 200: + case 201: + case 204: case 206: case 226: return CHANNEL_OK; @@ -1103,7 +1105,7 @@ static channel_op_res_t channel_put_method(channel_t *this, void *data) CURLcode curlrc = curl_easy_perform(channel_curl->handle); if (curlrc != CURLE_OK) { - ERROR("Channel put operation failed (%d): '%s'", curlrc, + ERROR("Channel PUT operation failed (%d): '%s'", curlrc, curl_easy_strerror(curlrc)); result = channel_map_curl_error(curlrc); goto cleanup_header; @@ -1116,7 +1118,11 @@ static channel_op_res_t channel_put_method(channel_t *this, void *data) if (channel_data->nocheckanswer) goto cleanup_header; - channel_log_reply(result, channel_data, NULL); + channel_log_reply(result, channel_data, &outdata); + + if (result == CHANNEL_OK) { + result = parse_reply(channel_data, &outdata); + } cleanup_header: outdata.memory != NULL ? free(outdata.memory) : (void)0; diff --git a/suricatta/server_general.c b/suricatta/server_general.c index 1c8e777e..6f4bce8b 100644 --- a/suricatta/server_general.c +++ b/suricatta/server_general.c @@ -324,6 +324,9 @@ static void *server_progress_thread (void *data) if (logbuffer) { channel_data.request_body = logbuffer; channel_data.method = CHANNEL_PUT; + /* .format is already specified in channel_data_defaults, + * but being explicit doesn't hurt. */ + channel_data.format = CHANNEL_PARSE_NONE; channel_data.content_type = "application/text"; result = map_channel_retcode(channel->put(channel, (void *)&channel_data)); if (result != SERVER_OK) diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c index 726ef191..a400c12c 100644 --- a/suricatta/server_hawkbit.c +++ b/suricatta/server_hawkbit.c @@ -1590,6 +1590,7 @@ static server_op_res_t server_send_target_data(void) } channel_data_reply.url = url; + channel_data_reply.format = CHANNEL_PARSE_NONE; channel_data_reply.request_body = json_reply_string; TRACE("URL=%s JSON=%s", channel_data_reply.url, channel_data_reply.request_body); channel_data_reply.method = CHANNEL_PUT; diff --git a/suricatta/server_lua.c b/suricatta/server_lua.c index 579ddc70..bdf34084 100644 --- a/suricatta/server_lua.c +++ b/suricatta/server_lua.c @@ -717,49 +717,45 @@ static int channel_do_operation(lua_State *L, channel_method_t op) push_result(L, result); lua_pushnumber(L, result); lua_newtable(L); - if (op == CHANNEL_GET || channel_data.method == CHANNEL_POST || - channel_data.method == CHANNEL_PATCH) { - push_to_table(L, "http_response_code", channel_data.http_response_code); - push_to_table(L, "format", channel_data.format); - #ifdef CONFIG_JSON - if (channel_data.format == CHANNEL_PARSE_JSON) { - lua_pushstring(L, "json_reply"); - if (!channel_data.json_reply || - !json_to_table(L, channel_data.json_reply)) { - lua_pushnil(L); - } - lua_settable(L, -3); - - if (channel_data.json_reply && - json_object_put(channel_data.json_reply) != 1) { - ERROR("JSON object should be freed but was not."); - } - } - #endif - if (channel_data.format == CHANNEL_PARSE_RAW) { - lua_pushstring(L, "raw_reply"); - if (!channel_data.raw_reply) { - lua_pushnil(L); - } else { - lua_pushstring(L, channel_data.raw_reply); - free(channel_data.raw_reply); - } - lua_settable(L, -3); + push_to_table(L, "http_response_code", channel_data.http_response_code); + push_to_table(L, "format", channel_data.format); + #ifdef CONFIG_JSON + if (channel_data.format == CHANNEL_PARSE_JSON) { + lua_pushstring(L, "json_reply"); + if (!channel_data.json_reply || + !json_to_table(L, channel_data.json_reply)) { + lua_pushnil(L); } + lua_settable(L, -3); - lua_pushstring(L, "received_headers"); - lua_newtable(L); - if (!LIST_EMPTY(channel_data.received_headers)) { - struct dict_entry *entry; - LIST_FOREACH(entry, channel_data.received_headers, next) { - lua_pushstring(L, dict_entry_get_key(entry)); - lua_pushstring(L, dict_entry_get_value(entry)); - lua_settable(L, -3); - } + if (channel_data.json_reply && + json_object_put(channel_data.json_reply) != 1) { + ERROR("JSON object should be freed but was not."); + } + } + #endif + if (channel_data.format == CHANNEL_PARSE_RAW) { + lua_pushstring(L, "raw_reply"); + if (!channel_data.raw_reply) { + lua_pushnil(L); + } else { + lua_pushstring(L, channel_data.raw_reply); + free(channel_data.raw_reply); } lua_settable(L, -3); } + lua_pushstring(L, "received_headers"); + lua_newtable(L); + if (!LIST_EMPTY(channel_data.received_headers)) { + struct dict_entry *entry; + LIST_FOREACH(entry, channel_data.received_headers, next) { + lua_pushstring(L, dict_entry_get_key(entry)); + lua_pushstring(L, dict_entry_get_value(entry)); + lua_settable(L, -3); + } + } + lua_settable(L, -3); dict_drop_db(&header_send); dict_drop_db(&header_receive);
PUT may return the sent document applied. This is currently not returned to channel_curl consumers. Signed-off-by: Christian Storm <christian.storm@siemens.com> --- corelib/channel_curl.c | 10 ++++-- suricatta/server_general.c | 3 ++ suricatta/server_hawkbit.c | 1 + suricatta/server_lua.c | 70 ++++++++++++++++++-------------------- 4 files changed, 45 insertions(+), 39 deletions(-)