PHP: allowed to specify URLs without a trailing '/'.

Both @lucatacconi & @mwoodpatrick reported what appears to be the same
issue on GitHub. Namely that when using the PHP language module and
trying to access a URL that is a directory but without specifying the
trailing '/', they were getting a '503 Service Unavailable' error.

Note: This is when _not_ using the 'script' option.

E.g with the following config

  {
      "listeners": {
          "[::1]:8080": {
              "pass": "applications/php"
          }
      },

      "applications": {
          "php": {
              "type": "php",
              "root": "/var/tmp/unit-php"
          }
      }
  }

and with a directory path of /var/tmp/unit-php/foo containing an
index.php, you would see the following

  $ curl http://localhost/foo
  <title>Error 503</title>
  Error 503

However

  $ curl http://localhost/foo/

would work and serve up the index.php

This commit fixes the above so you get the desired behaviour without
specifying the trailing '/' by doing the following

  1] If the URL doesn't end in .php and doesn't have a trailing '/'
     then check if the requested path is a directory.

  2) If it is a directory then create a 301 re-direct pointing to it.
     This matches the behaviour of the likes of nginx, Apache and
     lighttpd.

     This also matches the behaviour of the "share" action in Unit.

This doesn't effect the behaviour of the 'script' option which bypasses
the nxt_php_dynamic_request() function.

This also adds a couple of tests to test/test_php_application.py to
ensure this continues to work.

Closes: <https://github.com/nginx/unit/issues/717>
Closes: <https://github.com/nginx/unit/issues/753>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>

5 files changed, 143 insertions, 6 deletions

diff --git a/docs/changes.xml b/docs/changes.xml
index 59c55fea..014f243d 100644
--- a/docs/changes.xml
+++ b/docs/changes.xml
@@ -85,6 +85,13 @@ fix error in connection statistics when using proxy.
 </para>
 </change>
 
+<change type="bugfix">
+<para>
+PHP directory URLs without a trailing '/' would give a 503 error (fixed with
+a 301 re-direct).
+</para>
+</change>
+
 </changes>
 
 
diff --git a/src/nxt_php_sapi.c b/src/nxt_php_sapi.c
index ca49b289..924dd4e6 100644
--- a/src/nxt_php_sapi.c
+++ b/src/nxt_php_sapi.c
@@ -14,6 +14,7 @@
 #include <nxt_router.h>
 #include <nxt_unit.h>
 #include <nxt_unit_request.h>
+#include <nxt_http.h>
 
 
 #if (PHP_VERSION_ID >= 50400)
@@ -100,6 +101,8 @@ static void nxt_php_str_trim_trail(nxt_str_t *str, u_char t);
 static void nxt_php_str_trim_lead(nxt_str_t *str, u_char t);
 nxt_inline u_char *nxt_realpath(const void *c);
 
+static nxt_int_t nxt_php_do_301(nxt_unit_request_info_t *req);
+
 static void nxt_php_request_handler(nxt_unit_request_info_t *req);
 static void nxt_php_dynamic_request(nxt_php_run_ctx_t *ctx,
     nxt_unit_request_t *r);
@@ -920,6 +923,63 @@ nxt_realpath(const void *c)
 }
 
 
+static nxt_int_t
+nxt_php_do_301(nxt_unit_request_info_t *req)
+{
+    char                *p, *url, *port;
+    uint32_t            size;
+    const char          *proto;
+    nxt_unit_request_t  *r;
+
+    r = req->request;
+
+    url = nxt_malloc(sizeof("https://") - 1
+                     + r->server_name_length
+                     + r->local_port_length + 1
+                     + r->path_length + 1
+                     + r->query_length + 1
+                     + 1);
+    if (nxt_slow_path(url == NULL)) {
+        return NXT_UNIT_ERROR;
+    }
+
+    proto = r->tls ? "https://" : "http://";
+    p = nxt_cpymem(url, proto, strlen(proto));
+    p = nxt_cpymem(p, nxt_unit_sptr_get(&r->server_name),
+                   r->server_name_length);
+
+    port = nxt_unit_sptr_get(&r->local_port);
+    if (r->local_port_length > 0
+        && !(r->tls && strcmp(port, "443") == 0)
+        && !(!r->tls && strcmp(port, "80") == 0))
+    {
+        *p++ = ':';
+        p = nxt_cpymem(p, port, r->local_port_length);
+    }
+
+    p = nxt_cpymem(p, nxt_unit_sptr_get(&r->path), r->path_length);
+    *p++ = '/';
+
+    if (r->query_length > 0) {
+        *p++ = '?';
+        p = nxt_cpymem(p, nxt_unit_sptr_get(&r->query), r->query_length);
+    }
+
+    *p = '\0';
+
+    size = p - url;
+
+    nxt_unit_response_init(req, NXT_HTTP_MOVED_PERMANENTLY, 1,
+                           nxt_length("Location") + size);
+    nxt_unit_response_add_field(req, "Location", nxt_length("Location"),
+                                url, size);
+
+    nxt_free(url);
+
+    return NXT_UNIT_OK;
+}
+
+
 static void
 nxt_php_request_handler(nxt_unit_request_info_t *req)
 {
@@ -975,15 +1035,33 @@ nxt_php_dynamic_request(nxt_php_run_ctx_t *ctx, nxt_unit_request_t *r)
     } else if (path.start[path.length - 1] == '/') {
         script_name = *ctx->index;
 
-    } else {
-        if (nxt_slow_path(path.length < 4
-                          || nxt_memcmp(path.start + (path.length - 4),
-                                        ".php", 4)))
-        {
-            nxt_unit_request_done(ctx->req, NXT_UNIT_ERROR);
+    } else if (path.length < 4
+               || nxt_memcmp(path.start + (path.length - 4), ".php", 4) != 0)
+    {
+        char         tpath[PATH_MAX];
+        nxt_int_t    ec;
+        struct stat  sb;
+
+        ec = NXT_UNIT_ERROR;
+
+        if (ctx->root->length + path.length + 1 > PATH_MAX) {
+            nxt_unit_request_done(ctx->req, ec);
 
             return;
         }
+
+        p = nxt_cpymem(tpath, ctx->root->start, ctx->root->length);
+        p = nxt_cpymem(p, path.start, path.length);
+        *p = '\0';
+
+        ret = stat(tpath, &sb);
+        if (ret == 0 && S_ISDIR(sb.st_mode)) {
+            ec = nxt_php_do_301(ctx->req);
+        }
+
+        nxt_unit_request_done(ctx->req, ec);
+
+        return;
     }
 
     ctx->script_filename.length = ctx->root->length
diff --git a/src/nxt_router.c b/src/nxt_router.c
index 6430aea4..cfd79bf5 100644
--- a/src/nxt_router.c
+++ b/src/nxt_router.c
@@ -5254,6 +5254,12 @@ nxt_router_prepare_msg(nxt_task_t *task, nxt_http_request_t *r,
     p = nxt_cpymem(p, nxt_sockaddr_address(r->local), r->local->address_length);
     *p++ = '\0';
 
+    req->local_port_length = nxt_sockaddr_port_length(r->local);
+    nxt_unit_sptr_set(&req->local_port, p);
+    p = nxt_cpymem(p, nxt_sockaddr_port(r->local),
+                   nxt_sockaddr_port_length(r->local));
+    *p++ = '\0';
+
     req->tls = r->tls;
     req->websocket_handshake = r->websocket_handshake;
 
diff --git a/src/nxt_unit_request.h b/src/nxt_unit_request.h
index f6b96838..a6ebf0b6 100644
--- a/src/nxt_unit_request.h
+++ b/src/nxt_unit_request.h
@@ -19,6 +19,7 @@ struct nxt_unit_request_s {
     uint8_t               version_length;
     uint8_t               remote_length;
     uint8_t               local_addr_length;
+    uint8_t               local_port_length;
     uint8_t               tls;
     uint8_t               websocket_handshake;
     uint8_t               app_target;
@@ -39,6 +40,7 @@ struct nxt_unit_request_s {
     nxt_unit_sptr_t       version;
     nxt_unit_sptr_t       remote;
     nxt_unit_sptr_t       local_addr;
+    nxt_unit_sptr_t       local_port;
     nxt_unit_sptr_t       server_name;
     nxt_unit_sptr_t       target;
     nxt_unit_sptr_t       path;
diff --git a/test/test_php_application.py b/test/test_php_application.py
index f1dcc995..f442f551 100644
--- a/test/test_php_application.py
+++ b/test/test_php_application.py
@@ -4,6 +4,7 @@ import re
 import shutil
 import signal
 import time
+from pathlib import Path
 
 import pytest
 from unit.applications.lang.php import TestApplicationPHP
@@ -620,6 +621,49 @@ opcache.preload_user = %(user)s
         assert resp['status'] == 200, 'status'
         assert resp['body'] != '', 'body not empty'
 
+    def test_php_application_trailing_slash(self, temp_dir):
+        new_root = temp_dir + "/php-root"
+        os.makedirs(new_root + '/path')
+
+        Path(new_root + '/path/index.php').write_text('<?php echo "OK\n"; ?>')
+
+        addr = temp_dir + '/sock'
+
+        assert 'success' in self.conf(
+            {
+                "listeners": {
+                    "*:7080": {"pass": "applications/php-path"},
+                    "unix:" + addr: {"pass": "applications/php-path"},
+                },
+                "applications": {
+                    "php-path": {
+                        "type": self.get_application_type(),
+                        "processes": {"spare": 0},
+                        "root": new_root,
+                    }
+                },
+            }
+        ), 'configure trailing slash'
+
+        assert self.get(url='/path/')['status'] == 200, 'uri with trailing /'
+
+        resp = self.get(url='/path?q=a')
+        assert resp['status'] == 301, 'uri without trailing /'
+        assert (
+            resp['headers']['Location'] == 'http://localhost:7080/path/?q=a'
+        ), 'Location with query string'
+
+        resp = self.get(
+            sock_type='unix',
+            addr=addr,
+            url='/path',
+            headers={'Host': 'foo', 'Connection': 'close'},
+        )
+        assert resp['status'] == 301, 'uri without trailing /'
+        assert (
+            resp['headers']['Location'] == 'http://foo/path/'
+        ), 'Location with custom Host over UDS'
+
     def test_php_application_extension_check(self, temp_dir):
         self.load('phpinfo')