From d87b873ea5bbdae1352b2a61f5110f20b65928cc Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Sun, 12 Jan 2025 08:07:20 -0800 Subject: [PATCH] http: status and reason fixes (make it match docs) --- docs/ref/api/http.md | 2 +- include/nng/http.h | 2 +- src/supplemental/http/http_api.h | 2 +- src/supplemental/http/http_client.c | 1 + src/supplemental/http/http_conn.c | 61 ++++++++++++++--------------- src/supplemental/http/http_msg.c | 15 +------ src/supplemental/http/http_msg.h | 2 - src/supplemental/http/http_public.c | 5 +-- src/supplemental/http/http_server.c | 1 + 9 files changed, 38 insertions(+), 53 deletions(-) diff --git a/docs/ref/api/http.md b/docs/ref/api/http.md index b2382155d..48b09300c 100644 --- a/docs/ref/api/http.md +++ b/docs/ref/api/http.md @@ -108,7 +108,7 @@ by the server in the last exchange on _conn_. (If no exchange has been performed A descriptive message matching the status code is returned by {{i:`nng_http_get_reason`}}. -The {{i:`nng_http_set_status`}} function is used on a server in a handler callback to set the status codethat will be +The {{i:`nng_http_set_status`}} function is used on a server in a handler callback to set the status code that will be reported to the client to _status_, and the associated text (reason) to _reason_. If _reason_ is `NULL`, then a built in reason based on the _status_ will be used instead. diff --git a/include/nng/http.h b/include/nng/http.h index 69e57fa5e..d86548155 100644 --- a/include/nng/http.h +++ b/include/nng/http.h @@ -162,7 +162,7 @@ NNG_DECL const char *nng_http_get_reason(nng_http *); // nng_http_set_status sets the status for the transaction (server API), // and also sets the reason (message) for it. (If NULL is used for the reason, // then a builtin value is used based on the code.) -NNG_DECL int nng_http_set_status(nng_http *, uint16_t, const char *); +NNG_DECL void nng_http_set_status(nng_http *, uint16_t, const char *); // nng_http_set_version is used to change the version of a request. // Normally the version is "HTTP/1.1". Note that the framework does diff --git a/src/supplemental/http/http_api.h b/src/supplemental/http/http_api.h index 8ebe74bb7..02b5fbe45 100644 --- a/src/supplemental/http/http_api.h +++ b/src/supplemental/http/http_api.h @@ -359,7 +359,7 @@ extern int nni_http_set_version(nng_http *conn, const char *vers); extern void nni_http_set_method(nng_http *conn, const char *method); extern const char *nni_http_get_method(nng_http *conn); -extern int nni_http_set_status( +extern void nni_http_set_status( nng_http *conn, uint16_t status, const char *reason); extern uint16_t nni_http_get_status(nng_http *); diff --git a/src/supplemental/http/http_client.c b/src/supplemental/http/http_client.c index 4622dd947..4247f4b16 100644 --- a/src/supplemental/http/http_client.c +++ b/src/supplemental/http/http_client.c @@ -401,6 +401,7 @@ nni_http_transact_conn(nni_http_conn *conn, nni_aio *aio) txn->state = HTTP_SENDING; nni_http_res_reset(txn->res); + nni_http_set_status(txn->conn, 0, NULL); nni_mtx_lock(&http_txn_lk); if (!nni_aio_start(aio, http_txn_cancel, txn)) { diff --git a/src/supplemental/http/http_conn.c b/src/supplemental/http/http_conn.c index f3e023145..824baa96d 100644 --- a/src/supplemental/http/http_conn.c +++ b/src/supplemental/http/http_conn.c @@ -62,16 +62,19 @@ struct nng_http_conn { nng_http_req req; nng_http_res res; + uint16_t code; char meth[32]; char host[260]; // 253 per IETF, plus 6 for :port plus null char ubuf[200]; // Most URIs are smaller than this const char *vers; char *uri; - uint8_t *buf; - size_t bufsz; - size_t rd_get; - size_t rd_put; - size_t rd_discard; + char *rsn; + + uint8_t *buf; + size_t bufsz; + size_t rd_get; + size_t rd_put; + size_t rd_discard; enum read_flavor rd_flavor; enum write_flavor wr_flavor; @@ -580,6 +583,7 @@ nni_http_conn_reset(nng_http *conn) } conn->uri = NULL; nni_http_set_version(conn, NNG_HTTP_VERSION_1_1); + nni_http_set_status(conn, 0, NULL); } void @@ -864,10 +868,10 @@ nni_http_get_method(nng_http *conn) uint16_t nni_http_get_status(nng_http *conn) { - if (conn->res.code == 0) { + if (conn->code == 0) { return (NNG_HTTP_STATUS_OK); } - return (conn->res.code); + return (conn->code); } const char * @@ -973,39 +977,34 @@ nni_http_reason(uint16_t code) const char * nni_http_get_reason(nng_http *conn) { -#ifdef NNG_SUPP_HTTP - return ( - conn->res.rsn ? conn->res.rsn : nni_http_reason(conn->res.code)); -#else - return (NULL); -#endif + return (conn->rsn ? conn->rsn : nni_http_reason(conn->code)); } -int +void nni_http_set_status(nng_http *conn, uint16_t status, const char *reason) { - conn->res.code = status; - char *dup = NULL; - if ((reason != NULL) && - (strcmp(reason, nni_http_reason(conn->res.code)) == 0)) { - reason = NULL; - return (0); - } - if ((reason != NULL) && (dup = nni_strdup(reason)) == NULL) { - return (NNG_ENOMEM); - } - if (conn->res.rsn != NULL) { - nni_strfree(conn->res.rsn); + conn->code = status; + char *dup = NULL; + if (reason != NULL) { + if (strcmp(reason, nni_http_reason(conn->code)) == 0) { + dup = NULL; + } else { + // This might fail, but if it does, we will just + // use the built in reason, which should not + // fundamentally affect semantics. This allows us to + // make this function void, and avoid some error + // handling. + dup = nni_strdup(reason); + } } - conn->res.rsn = dup; - return (0); + nni_strfree(conn->rsn); + conn->rsn = dup; } static int http_conn_set_error(nng_http *conn, uint16_t status, const char *reason, const char *body, const char *redirect) { - int rv; char content[1024]; const char *prefix = "\n" "%d %s\n" @@ -1026,9 +1025,7 @@ http_conn_set_error(nng_http *conn, uint16_t status, const char *reason, conn->res.iserr = true; - if ((rv = nni_http_set_status(conn, status, reason)) != 0) { - return (rv); - } + nni_http_set_status(conn, status, reason); reason = nni_http_get_reason(conn); if (body == NULL) { diff --git a/src/supplemental/http/http_msg.c b/src/supplemental/http/http_msg.c index 5cc71b7dc..009c8ab41 100644 --- a/src/supplemental/http/http_msg.c +++ b/src/supplemental/http/http_msg.c @@ -72,9 +72,6 @@ void nni_http_res_reset(nni_http_res *res) { http_entity_reset(&res->data); - nni_strfree(res->rsn); - res->rsn = NULL; - res->code = 0; } // http_entity_set_data sets the entity, but does not update the @@ -183,8 +180,6 @@ nni_http_res_init(nni_http_res *res) res->data.data = NULL; res->data.size = 0; res->data.own = false; - res->rsn = NULL; - res->code = 0; } static int @@ -252,7 +247,6 @@ http_req_parse_line(nng_http *conn, void *line) static int http_res_parse_line(nng_http *conn, uint8_t *line) { - int rv; char *reason; char *codestr; char *version; @@ -276,14 +270,9 @@ http_res_parse_line(nng_http *conn, uint8_t *line) return (NNG_EPROTO); } - if ((rv = nni_http_set_status(conn, (uint16_t) status, reason)) != 0) { - return (rv); - } + nni_http_set_status(conn, (uint16_t) status, reason); - if ((rv = nni_http_set_version(conn, version)) != 0) { - return (rv); - } - return (0); + return (nni_http_set_version(conn, version)); } // nni_http_req_parse parses a request (but not any attached entity data). diff --git a/src/supplemental/http/http_msg.h b/src/supplemental/http/http_msg.h index 038d9a040..5a13c409f 100644 --- a/src/supplemental/http/http_msg.h +++ b/src/supplemental/http/http_msg.h @@ -49,8 +49,6 @@ struct nng_http_req { struct nng_http_res { nni_http_entity data; - uint16_t code; - char *rsn; bool iserr; http_header location; }; diff --git a/src/supplemental/http/http_public.c b/src/supplemental/http/http_public.c index 55c54394e..8210224bb 100644 --- a/src/supplemental/http/http_public.c +++ b/src/supplemental/http/http_public.c @@ -138,16 +138,15 @@ nng_http_get_version(nng_http *conn) #endif } -int +void nng_http_set_status(nng_http *conn, uint16_t status, const char *reason) { #ifdef NNG_SUPP_HTTP - return (nni_http_set_status(conn, status, reason)); + nni_http_set_status(conn, status, reason); #else NNI_ARG_UNUSED(res); NNI_ARG_UNUSED(status); NNI_ARG_UNUSED(reason); - return (NNG_ENOTSUP); #endif } diff --git a/src/supplemental/http/http_server.c b/src/supplemental/http/http_server.c index 7a7caeca5..ffb014490 100644 --- a/src/supplemental/http/http_server.c +++ b/src/supplemental/http/http_server.c @@ -704,6 +704,7 @@ http_sconn_rxdone(void *arg) // make sure the response is freshly initialized nni_http_res_reset(nni_http_conn_res(sc->conn)); nni_http_set_version(sc->conn, NNG_HTTP_VERSION_1_1); + nni_http_set_status(sc->conn, 0, NULL); h->cb(sc->conn, h->data, &sc->cbaio); }