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 <https://www.rfc-editor.org/rfc/rfc2616#section-14.30>, which cannot be an
empty string.  However, the current RFC 7231 allows the Location to be a
relative URI <https://www.rfc-editor.org/rfc/rfc7231#section-7.1.2>, and a
relative URI may be an empty string <https://stackoverflow.com/a/43338457>.

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}

4 files changed, 33 insertions, 30 deletions

diff --git a/docs/changes.xml b/docs/changes.xml
index af963bbb..98650051 100644
--- a/docs/changes.xml
+++ b/docs/changes.xml
@@ -37,6 +37,12 @@ Ruby Rack environment parameter "SCRIPT_NAME" support.
 </para>
 </change>
 
+<change type="change">
+<para>
+supporting empty strings in the "location" option of the "return" action.
+</para>
+</change>
+
 <change type="feature">
 <para>
 variables support in the "location" option of the "return" action.
diff --git a/src/nxt_http.h b/src/nxt_http.h
index 7cef1345..36bc089d 100644
--- a/src/nxt_http.h
+++ b/src/nxt_http.h
@@ -206,7 +206,7 @@ typedef struct nxt_http_route_addr_rule_s  nxt_http_route_addr_rule_t;
 typedef struct {
     nxt_conf_value_t                *pass;
     nxt_conf_value_t                *ret;
-    nxt_str_t                       location;
+    nxt_conf_value_t                *location;
     nxt_conf_value_t                *proxy;
     nxt_conf_value_t                *share;
     nxt_str_t                       chroot;
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;
diff --git a/src/nxt_http_route.c b/src/nxt_http_route.c
index c2321906..af28400d 100644
--- a/src/nxt_http_route.c
+++ b/src/nxt_http_route.c
@@ -626,7 +626,7 @@ static nxt_conf_map_t  nxt_http_route_action_conf[] = {
     },
     {
         nxt_string("location"),
-        NXT_CONF_MAP_STR,
+        NXT_CONF_MAP_PTR,
         offsetof(nxt_http_action_conf_t, location)
     },
     {