From 6fb7777ce73ba529327d307ca0722e66a7cb9262 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 5 Apr 2022 11:47:56 +0200 Subject: Supporting variables in "location". ............ Description: ............ Before this commit, the encoded URI could be calculated at configuration time. Now, since variables can only be resolved at request time, we have different situations: - "location" contains no variables: In this case, we still encode the URI in the conf structure, at configuration time, and then we just copy the resulting string to the ctx structure at request time. - "location" contains variables: In this case, we compile the var string at configure time, then when we resolve it at request time, and then we encode the string. In both cases, as was being done before, if the string is empty, either before or after resolving variables, we skip the encoding. ........... Usefulness: ........... An example of why this feature may be useful is redirecting HTTP to HTTPS with something like: "action": { "return": 301, "location": "https://${host}${uri}" } ..... Bugs: ..... This feature conflicts with the relevant RFCs in the following: '$' is used for Unit variables, but '$' is a reserved character in a URI, to be used as a sub-delimiter. However, it's almost never used as that, and in fact, other parts of Unit already conflict with '$' being a reserved character for use as a sub-delimiter, so this is at least consistent in that sense. VBart suggested an easy workaround if we ever need it: adding a variable '$sign' which resolves to a literal '$'. ...... Notes: ...... An empty string is handled as if "location" wasn't specified at all, so no Location header is sent. This is incorrect, and the code is slightly misleading. The Location header consists of a URI-reference[1], which might be a relative one, which itself might consist of an empty string[2]. [1]: [2]: Now that we have variables, it's more likely that an empty Location header will be requested, and we should handle it correctly. I think in a future commit we should modify the code to allow differentiating between an unset "location" and an empty one, which should be treated as any other "location" string. ................. Testing (manual): ................. { "listeners": { "*:80": { "pass": "routes/str" }, "*:81": { "pass": "routes/empty" }, "*:82": { "pass": "routes/var" }, "*:83": { "pass": "routes/enc-str" }, "*:84": { "pass": "routes/enc-var" } }, "routes": { "str": [ { "action": { "return": 301, "location": "foo" } } ], "empty": [ { "action": { "return": 301, "location": "" } } ], "var": [ { "action": { "return": 301, "location": "$host" } } ], "enc-str": [ { "action": { "return": 301, "location": "f%23o#o" } } ], "enc-var": [ { "action": { "return": 301, "location": "f%23o${host}#o" } } ] } } $ curl --dump-header - localhost:80 HTTP/1.1 301 Moved Permanently Location: foo Server: Unit/1.27.0 Date: Thu, 07 Apr 2022 23:30:06 GMT Content-Length: 0 $ curl --dump-header - localhost:81 HTTP/1.1 301 Moved Permanently Server: Unit/1.27.0 Date: Thu, 07 Apr 2022 23:30:08 GMT Content-Length: 0 $ curl --dump-header - localhost:82 HTTP/1.1 301 Moved Permanently Location: localhost Server: Unit/1.27.0 Date: Thu, 07 Apr 2022 23:30:15 GMT Content-Length: 0 $ curl --dump-header - -H "Host: bar" localhost:82 HTTP/1.1 301 Moved Permanently Location: bar Server: Unit/1.27.0 Date: Thu, 07 Apr 2022 23:30:23 GMT Content-Length: 0 $ curl --dump-header - -H "Host: " localhost:82 HTTP/1.1 301 Moved Permanently Server: Unit/1.27.0 Date: Thu, 07 Apr 2022 23:30:29 GMT Content-Length: 0 $ curl --dump-header - localhost:83 HTTP/1.1 301 Moved Permanently Location: f%23o#o Server: Unit/1.27.0 Date: Sat, 09 Apr 2022 11:22:23 GMT Content-Length: 0 $ curl --dump-header - -H "Host: " localhost:84 HTTP/1.1 301 Moved Permanently Location: f%23o#o Server: Unit/1.27.0 Date: Sat, 09 Apr 2022 11:22:44 GMT Content-Length: 0 $ curl --dump-header - -H "Host: alx" localhost:84 HTTP/1.1 301 Moved Permanently Location: f%23oalx#o Server: Unit/1.27.0 Date: Sat, 09 Apr 2022 11:22:52 GMT Content-Length: 0 $ curl --dump-header - -H "Host: a#l%23x" localhost:84 HTTP/1.1 301 Moved Permanently Location: f%2523oa#l%2523x%23o Server: Unit/1.27.0 Date: Sat, 09 Apr 2022 11:23:09 GMT Content-Length: 0 $ curl --dump-header - -H "Host: b##ar" localhost:82 HTTP/1.1 301 Moved Permanently Location: b#%23ar Server: Unit/1.27.0 Date: Sat, 09 Apr 2022 11:25:01 GMT Content-Length: 0 --- src/nxt_http_return.c | 183 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 146 insertions(+), 37 deletions(-) (limited to 'src/nxt_http_return.c') diff --git a/src/nxt_http_return.c b/src/nxt_http_return.c index 18fd490d..92dfa465 100644 --- a/src/nxt_http_return.c +++ b/src/nxt_http_return.c @@ -8,13 +8,26 @@ typedef struct { - nxt_http_status_t status; - nxt_str_t location; + nxt_http_status_t status; + nxt_var_t *location; + nxt_str_t encoded; + uint8_t loc_is_const; } nxt_http_return_conf_t; +typedef struct { + nxt_http_action_t *action; + nxt_str_t location; + nxt_str_t encoded; +} nxt_http_return_ctx_t; + + static nxt_http_action_t *nxt_http_return(nxt_task_t *task, nxt_http_request_t *r, nxt_http_action_t *action); +static nxt_int_t nxt_http_return_encode(nxt_mp_t *mp, nxt_str_t *encoded, + const nxt_str_t *location); +static void nxt_http_return_send_ready(nxt_task_t *task, void *obj, void *data); +static void nxt_http_return_var_error(nxt_task_t *task, void *obj, void *data); static const nxt_http_request_state_t nxt_http_return_send_state; @@ -24,8 +37,8 @@ nxt_int_t nxt_http_return_init(nxt_mp_t *mp, nxt_http_action_t *action, nxt_http_action_conf_t *acf) { - nxt_str_t *loc; - nxt_uint_t encode; + nxt_str_t str; + nxt_var_t *var; nxt_http_return_conf_t *conf; conf = nxt_mp_zget(mp, sizeof(nxt_http_return_conf_t)); @@ -38,30 +51,22 @@ nxt_http_return_init(nxt_mp_t *mp, nxt_http_action_t *action, conf->status = nxt_conf_get_number(acf->ret); - if (acf->location.length > 0) { - if (nxt_is_complex_uri_encoded(acf->location.start, - acf->location.length)) - { - loc = nxt_str_dup(mp, &conf->location, &acf->location); - if (nxt_slow_path(loc == NULL)) { - return NXT_ERROR; - } - - } else { - loc = &conf->location; - - encode = nxt_encode_complex_uri(NULL, acf->location.start, - acf->location.length); - loc->length = acf->location.length + encode * 2; - - loc->start = nxt_mp_nget(mp, loc->length); - if (nxt_slow_path(loc->start == NULL)) { - return NXT_ERROR; - } - - nxt_encode_complex_uri(loc->start, acf->location.start, - acf->location.length); - } + if (acf->location.length == 0) { + conf->loc_is_const = 1; + return NXT_OK; + } + + var = nxt_var_compile(&acf->location, mp, 0); + if (nxt_slow_path(var == NULL)) { + return NXT_ERROR; + } + + conf->location = var; + conf->loc_is_const = nxt_var_is_const(var); + + if (conf->loc_is_const) { + nxt_var_raw(conf->location, &str); + return nxt_http_return_encode(mp, &conf->encoded, &str); } return NXT_OK; @@ -72,13 +77,21 @@ nxt_http_action_t * nxt_http_return(nxt_task_t *task, nxt_http_request_t *r, nxt_http_action_t *action) { - nxt_http_field_t *field; + nxt_int_t ret; + nxt_str_t loc; + nxt_http_return_ctx_t *ctx; nxt_http_return_conf_t *conf; conf = action->u.conf; - nxt_debug(task, "http return: %d (loc: \"%V\")", - conf->status, &conf->location); + if (conf->location == NULL) { + nxt_str_null(&loc); + + } else { + nxt_var_raw(conf->location, &loc); + } + + nxt_debug(task, "http return: %d (loc: \"%V\")", conf->status, &loc); if (conf->status >= NXT_HTTP_BAD_REQUEST && conf->status <= NXT_HTTP_SERVER_ERROR_MAX) @@ -87,27 +100,123 @@ nxt_http_return(nxt_task_t *task, nxt_http_request_t *r, return NULL; } + ctx = nxt_mp_zget(r->mem_pool, sizeof(nxt_http_return_ctx_t)); + if (nxt_slow_path(ctx == NULL)) { + goto fail; + } + + ctx->action = action; r->status = conf->status; r->resp.content_length_n = 0; - if (conf->location.length > 0) { + if (conf->loc_is_const) { + ctx->encoded = conf->encoded; + + nxt_http_return_send_ready(task, r, ctx); + + } else { + ret = nxt_var_query_init(&r->var_query, r, r->mem_pool); + if (nxt_slow_path(ret != NXT_OK)) { + goto fail; + } + + nxt_var_query(task, r->var_query, conf->location, &ctx->location); + + nxt_var_query_resolve(task, r->var_query, ctx, + nxt_http_return_send_ready, + nxt_http_return_var_error); + } + + return NULL; + +fail: + + nxt_http_request_error(task, r, NXT_HTTP_INTERNAL_SERVER_ERROR); + return NULL; +} + + +static nxt_int_t +nxt_http_return_encode(nxt_mp_t *mp, nxt_str_t *encoded, + const nxt_str_t *location) +{ + nxt_uint_t encode; + + if (nxt_is_complex_uri_encoded(location->start, location->length)) { + *encoded = *location; + + return NXT_OK; + } + + encode = nxt_encode_complex_uri(NULL, location->start, location->length); + encoded->length = location->length + encode * 2; + + encoded->start = nxt_mp_nget(mp, encoded->length); + if (nxt_slow_path(encoded->start == NULL)) { + return NXT_ERROR; + } + + nxt_encode_complex_uri(encoded->start, location->start, location->length); + + return NXT_OK; +} + + +static void +nxt_http_return_send_ready(nxt_task_t *task, void *obj, void *data) +{ + nxt_int_t ret; + nxt_http_field_t *field; + nxt_http_action_t *action; + nxt_http_request_t *r; + nxt_http_return_ctx_t *ctx; + nxt_http_return_conf_t *conf; + + r = obj; + ctx = data; + action = ctx->action; + conf = action->u.conf; + + if (!conf->loc_is_const) { + ret = nxt_http_return_encode(r->mem_pool, &ctx->encoded, + &ctx->location); + if (nxt_slow_path(ret == NXT_ERROR)) { + goto fail; + } + } + + if (ctx->encoded.length > 0) { field = nxt_list_zero_add(r->resp.fields); if (nxt_slow_path(field == NULL)) { - nxt_http_request_error(task, r, NXT_HTTP_INTERNAL_SERVER_ERROR); - return NULL; + goto fail; } nxt_http_field_name_set(field, "Location"); - field->value = conf->location.start; - field->value_length = conf->location.length; + field->value = ctx->encoded.start; + field->value_length = ctx->encoded.length; } r->state = &nxt_http_return_send_state; nxt_http_request_header_send(task, r, NULL, NULL); - return NULL; + return; + +fail: + + nxt_http_request_error(task, r, NXT_HTTP_INTERNAL_SERVER_ERROR); +} + + +static void +nxt_http_return_var_error(nxt_task_t *task, void *obj, void *data) +{ + nxt_http_request_t *r; + + r = obj; + + nxt_http_request_error(task, r, NXT_HTTP_INTERNAL_SERVER_ERROR); } -- cgit From 7066acb2ce438526fb0d60df443320d1c8366760 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Sat, 9 Apr 2022 21:27:12 +0200 Subject: Supporting empty Location URIs. An empty string in Location was being handled specially by not sending a Location header. This may occur after variable resolution, so we need to consider this scenario. The obsolete RFC 2616 defined the Location header as consisting of an absolute URI , which cannot be an empty string. However, the current RFC 7231 allows the Location to be a relative URI , and a relative URI may be an empty string . Due to these considerations, this patch allows sending an empty Location header without handling this case specially. This behavior will probably be more straightforward to users, too. It also simplifies the code, which is now more readable, fast, and conformant to the current RFC. We're skipping an allocation at request time in a common case such as "action": {"return": 404} --- src/nxt_http_return.c | 53 ++++++++++++++++++++++++--------------------------- 1 file changed, 25 insertions(+), 28 deletions(-) (limited to 'src/nxt_http_return.c') diff --git a/src/nxt_http_return.c b/src/nxt_http_return.c index 92dfa465..e3dd02ad 100644 --- a/src/nxt_http_return.c +++ b/src/nxt_http_return.c @@ -11,12 +11,10 @@ typedef struct { nxt_http_status_t status; nxt_var_t *location; nxt_str_t encoded; - uint8_t loc_is_const; } nxt_http_return_conf_t; typedef struct { - nxt_http_action_t *action; nxt_str_t location; nxt_str_t encoded; } nxt_http_return_ctx_t; @@ -38,7 +36,6 @@ nxt_http_return_init(nxt_mp_t *mp, nxt_http_action_t *action, nxt_http_action_conf_t *acf) { nxt_str_t str; - nxt_var_t *var; nxt_http_return_conf_t *conf; conf = nxt_mp_zget(mp, sizeof(nxt_http_return_conf_t)); @@ -51,20 +48,18 @@ nxt_http_return_init(nxt_mp_t *mp, nxt_http_action_t *action, conf->status = nxt_conf_get_number(acf->ret); - if (acf->location.length == 0) { - conf->loc_is_const = 1; + if (acf->location == NULL) { return NXT_OK; } - var = nxt_var_compile(&acf->location, mp, 0); - if (nxt_slow_path(var == NULL)) { + nxt_conf_get_string(acf->location, &str); + + conf->location = nxt_var_compile(&str, mp, 0); + if (nxt_slow_path(conf->location == NULL)) { return NXT_ERROR; } - conf->location = var; - conf->loc_is_const = nxt_var_is_const(var); - - if (conf->loc_is_const) { + if (nxt_var_is_const(conf->location)) { nxt_var_raw(conf->location, &str); return nxt_http_return_encode(mp, &conf->encoded, &str); } @@ -100,17 +95,23 @@ nxt_http_return(nxt_task_t *task, nxt_http_request_t *r, return NULL; } - ctx = nxt_mp_zget(r->mem_pool, sizeof(nxt_http_return_ctx_t)); - if (nxt_slow_path(ctx == NULL)) { - goto fail; + if (conf->location == NULL) { + ctx = NULL; + + } else { + ctx = nxt_mp_zget(r->mem_pool, sizeof(nxt_http_return_ctx_t)); + if (nxt_slow_path(ctx == NULL)) { + goto fail; + } } - ctx->action = action; r->status = conf->status; r->resp.content_length_n = 0; - if (conf->loc_is_const) { - ctx->encoded = conf->encoded; + if (ctx == NULL || nxt_var_is_const(conf->location)) { + if (ctx != NULL) { + ctx->encoded = conf->encoded; + } nxt_http_return_send_ready(task, r, ctx); @@ -167,25 +168,21 @@ nxt_http_return_send_ready(nxt_task_t *task, void *obj, void *data) { nxt_int_t ret; nxt_http_field_t *field; - nxt_http_action_t *action; nxt_http_request_t *r; nxt_http_return_ctx_t *ctx; - nxt_http_return_conf_t *conf; r = obj; ctx = data; - action = ctx->action; - conf = action->u.conf; - if (!conf->loc_is_const) { - ret = nxt_http_return_encode(r->mem_pool, &ctx->encoded, - &ctx->location); - if (nxt_slow_path(ret == NXT_ERROR)) { - goto fail; + if (ctx != NULL) { + if (ctx->location.length > 0) { + ret = nxt_http_return_encode(r->mem_pool, &ctx->encoded, + &ctx->location); + if (nxt_slow_path(ret == NXT_ERROR)) { + goto fail; + } } - } - if (ctx->encoded.length > 0) { field = nxt_list_zero_add(r->resp.fields); if (nxt_slow_path(field == NULL)) { goto fail; -- cgit From ba20fa3939c1505866d715a5a1a43f61a4f8de17 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 May 2022 11:18:58 +0200 Subject: Fixed memcpy(dest, NULL, 0) Undefined Behavior. nxt_str_null() setted the loc.start pointer to NULL, which was being passed to memcpy(3) through nxt_debug(). That caused Undefined Behavior, so we now pass an empty string. --- src/nxt_http_return.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/nxt_http_return.c') diff --git a/src/nxt_http_return.c b/src/nxt_http_return.c index e3dd02ad..8832941f 100644 --- a/src/nxt_http_return.c +++ b/src/nxt_http_return.c @@ -80,7 +80,7 @@ nxt_http_return(nxt_task_t *task, nxt_http_request_t *r, conf = action->u.conf; if (conf->location == NULL) { - nxt_str_null(&loc); + nxt_str_set(&loc, ""); } else { nxt_var_raw(conf->location, &loc); -- cgit From 7662ec5f1bf27de981a8aa100ab2c5c3fa985269 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 May 2022 12:20:19 +0200 Subject: Wrapped debug code in '#if (NXT_DEBUG)'. --- src/nxt_http_return.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/nxt_http_return.c') diff --git a/src/nxt_http_return.c b/src/nxt_http_return.c index 8832941f..82c91568 100644 --- a/src/nxt_http_return.c +++ b/src/nxt_http_return.c @@ -73,12 +73,14 @@ nxt_http_return(nxt_task_t *task, nxt_http_request_t *r, nxt_http_action_t *action) { nxt_int_t ret; - nxt_str_t loc; nxt_http_return_ctx_t *ctx; nxt_http_return_conf_t *conf; conf = action->u.conf; +#if (NXT_DEBUG) + nxt_str_t loc; + if (conf->location == NULL) { nxt_str_set(&loc, ""); @@ -87,6 +89,7 @@ nxt_http_return(nxt_task_t *task, nxt_http_request_t *r, } nxt_debug(task, "http return: %d (loc: \"%V\")", conf->status, &loc); +#endif if (conf->status >= NXT_HTTP_BAD_REQUEST && conf->status <= NXT_HTTP_SERVER_ERROR_MAX) -- cgit