unit
unit copied to clipboard
PHP: request to app failed without tailing slash in URI
I'm getting a 503 error when trying to access a php app I don't see any issues reported in the unit log file and the php module is enabled. Is there any logging I can enable to see why I'm getting the error, any pointers as to what's causing the issue or best way to debug would be appreciated
curl http://localhost/example_php
Error 503.
config from unit is { "listeners": { ":80": { "pass": "routes" } }, "routes": [ { "match": { "uri": "/static" }, "action": { "share": "/projects/www/$uri" } }, { "match": { "uri": "/example_php" }, "action": { "pass": "applications/example_php" } } ], "applications": { "example_php": { "type": "php", "processes": 2, "root": "/projects/www/php/phpinfo-app", "index": "index.php" } } } log file from unit 2022/09/10 12:55:13 [info] 2095#2095 discovery started 2022/09/10 12:55:13 [notice] 2095#2095 module: java 11.0.15 "/usr/lib64/unit/modules/java11.unit.so" 2022/09/10 12:55:13 [notice] 2095#2095 module: java 1.8.0_332 "/usr/lib64/unit/modules/java8.unit.so" 2022/09/10 12:55:13 [notice] 2095#2095 module: perl 5.34.1 "/usr/lib64/unit/modules/perl.unit.so" 2022/09/10 12:55:13 [notice] 2095#2095 module: php 8.1.5 "/usr/lib64/unit/modules/php.unit.so" 2022/09/10 12:55:13 [notice] 2095#2095 module: python 3.10.4 "/usr/lib64/unit/modules/python3.10.unit.so" 2022/09/10 12:55:13 [notice] 2095#2095 module: ruby 3.1.2 "/usr/lib64/unit/modules/ruby.unit.so" 2022/09/10 12:55:13 [info] 2091#2091 controller started 2022/09/10 12:55:13 [notice] 2091#2091 process 2095 exited with code 0 2022/09/10 12:55:13 [info] 2097#2097 router started 2022/09/10 12:55:13 [info] 2097#2097 OpenSSL 3.0.5 5 Jul 2022, 30000050 2022/09/10 12:55:13 [info] 2098#2098 "example_php" prototype started 2022/09/10 12:55:13 [info] 2099#2099 "example_php" application started 2022/09/10 12:55:13 [info] 2100#2100 "example_php" application started
Good Morning @mwoodpatrick . This looks like a problem within your PHP code. As you can see from the logs the PHP application started successfully.
A quick thing that is unlikely the problem but can you please change your static file share configuration form /projects/www/$uri
to /projects/www$uri
.
https://unit.nginx.org/configuration/#handling-actions
According to our documentation, the $uri does contain the leading /
, so therefore there is no need to add it again.
May I ask you to add a access.log to your configuration and can you share this as well? https://unit.nginx.org/configuration/#access-log
If I change the static file path to remove the "/" I get the same result
For the PHP issue the access log shows:
127.0.0.1 - - [12/Sep/2022:08:28:45 -0700] "GET /example_php HTTP/1.1" 503 54 "-" "curl/7.82.0" 127.0.0.1 - - [12/Sep/2022:08:28:49 -0700] "GET /example_php/ HTTP/1.1" 200 85786 "-" "curl/7.82.0"
It seems to work if I terminate the URL with a "/" and if I don't I get the 503 error
I am seeing an error in the log file
2022/09/12 08:28:29 [alert] 19997#19997 failed to store current configuration
even though the access os working anyone know what this issue is or how best to debug
Needing to terminate the URL with a '/' seems to be intended, from the docs...
index | Filename appended to any URI paths ending with a slash; applies if script is omitted.
Thanks for the info, much appreciated. It would be helpful if the log had something clearer than just error 503, this seems rather obscure
I agree, this could probably be handled better.
IIRC (memory is likely a little hazy) in the early days of the web if the last component of a URL was a directory you had to use a trailing '/' to signify it as such.
Over time severs relaxed a little and you could omit the trailing '/' and the server would figure out if it was a directory or file.
I will need to do some tracing and digging through the source to see what exactly Unit is doing here.
@mwoodpatrick Here's a patch against current unit that allows to skip the trailing '/' on PHP URLs that are directories, if you'd like to give it a spin...
(updated to fix a potential buffer overflow)
diff --git a/src/nxt_php_sapi.c b/src/nxt_php_sapi.c
index 68ef07eb..6d2f320e 100644
--- a/src/nxt_php_sapi.c
+++ b/src/nxt_php_sapi.c
@@ -958,11 +958,28 @@ nxt_php_dynamic_request(nxt_php_run_ctx_t *ctx, nxt_unit_request_t *r)
{
u_char *p;
nxt_str_t path, script_name;
- nxt_int_t ret;
+ nxt_int_t ret, isdir;
path.length = r->path_length;
path.start = nxt_unit_sptr_get(&r->path);
+ isdir = 0;
+ if (path.start[path.length - 1] != '/'
+ && ctx->root->length + path.length < PATH_MAX)
+ {
+ char tpath[PATH_MAX];
+ struct stat sb;
+
+ 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)) {
+ isdir = 1;
+ }
+ }
+
nxt_str_null(&script_name);
ctx->path_info.start = (u_char *) strstr((char *) path.start, ".php/");
@@ -972,7 +989,7 @@ nxt_php_dynamic_request(nxt_php_run_ctx_t *ctx, nxt_unit_request_t *r)
ctx->path_info.length = r->path_length - path.length;
- } else if (path.start[path.length - 1] == '/') {
+ } else if (path.start[path.length - 1] == '/' || isdir == 1) {
script_name = *ctx->index;
} else {
@@ -988,6 +1005,7 @@ nxt_php_dynamic_request(nxt_php_run_ctx_t *ctx, nxt_unit_request_t *r)
ctx->script_filename.length = ctx->root->length
+ path.length
+ + isdir
+ script_name.length;
p = nxt_malloc(ctx->script_filename.length + 1);
@@ -1004,6 +1022,7 @@ nxt_php_dynamic_request(nxt_php_run_ctx_t *ctx, nxt_unit_request_t *r)
p = nxt_cpymem(p, ctx->root->start, ctx->root->length);
p = nxt_cpymem(p, path.start, path.length);
+ p = nxt_cpymem(p, "/", isdir);
if (script_name.length > 0) {
p = nxt_cpymem(p, script_name.start, script_name.length);
@ac000 linking for visibility. https://github.com/nginx/unit/issues/717 .Looks like the same root cause.
@ac000
With the patch, requests that don't end with /
will have to add one more syscall stat(...)
. And most requests are like this.
@mwoodpatrick Here's an preliminary version of an alternative fix for this issue. It uses a 301 redirect to point to the path with a trailing '/', if it's a directory. This is how the likes of nginx, Apache & lighttpd do it. (It also skips the check on .php URLs).
Hopefully the 301 redirect works for you... (curl -L ...)
diff --git a/src/nxt_php_sapi.c b/src/nxt_php_sapi.c
index 68ef07eb..dc308da2 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
@@ -920,6 +921,95 @@ nxt_realpath(const void *c)
}
+static char *
+nxt_php_create_location(const nxt_unit_request_info_t *req, uint32_t *size)
+{
+ char *p, *url;
+ nxt_str_t server_name, local_port, path, query;
+ const char *proto;
+
+ proto = (req->request->tls == 1) ? "https://" : "http://";
+
+ server_name.start = nxt_unit_sptr_get(&req->request->server_name);
+ server_name.length = req->request->server_name_length;
+ local_port.start = nxt_unit_sptr_get(&req->request->local_port);
+ local_port.length = req->request->local_port_length;
+ path.start = nxt_unit_sptr_get(&req->request->path);
+ path.length = req->request->path_length;
+ query.start = nxt_unit_sptr_get(&req->request->query);
+ query.length = req->request->query_length;
+
+ url = nxt_malloc(8 /* max length of proto */
+ + server_name.length
+ + 1 + local_port.length
+ + path.length + 1 /* for the '/' */
+ + query.length + 1 /* for the '?' */
+ + 1);
+ if (nxt_slow_path(url == NULL)) {
+ return NULL;
+ }
+
+ p = nxt_cpymem(url, proto, strlen(proto));
+ p = nxt_cpymem(p, server_name.start, server_name.length);
+
+ if (nxt_strcmp(local_port.start, "80") != 0
+ && nxt_strcmp(local_port.start, "443") != 0)
+ {
+ p = nxt_cpymem(p, ":", 1);
+ p = nxt_cpymem(p, local_port.start, local_port.length);
+ }
+
+ p = nxt_cpymem(p, path.start, path.length);
+ p = nxt_cpymem(p, "/", 1);
+
+ if (query.length > 0) {
+ p = nxt_cpymem(p, "?", 1);
+ p = nxt_cpymem(p, query.start, query.length);
+ }
+
+ *p = '\0';
+
+ *size = p - url;
+
+ return url;
+}
+
+
+static void
+nxt_php_do_err_response(nxt_unit_request_info_t *req,
+ nxt_http_status_t status)
+{
+ nxt_int_t ec;
+
+ ec = NXT_UNIT_OK;
+
+ switch (status) {
+ case NXT_HTTP_MOVED_PERMANENTLY:
+ char *url;
+ uint32_t size;
+
+ url = nxt_php_create_location(req, &size);
+ if (nxt_slow_path(url == NULL)) {
+ ec = NXT_UNIT_ERROR;
+ goto out_request_done;
+ }
+
+ nxt_unit_response_init(req, status, 1, nxt_length("Location") + size);
+ nxt_unit_response_add_field(req, "Location", nxt_length("Location"),
+ url, size);
+ nxt_free(url);
+ break;
+
+ default:
+ ec = NXT_UNIT_ERROR;
+ }
+
+out_request_done:
+
+ nxt_unit_request_done(req, ec);
+}
+
+
static void
nxt_php_request_handler(nxt_unit_request_info_t *req)
{
@@ -963,6 +1053,26 @@ nxt_php_dynamic_request(nxt_php_run_ctx_t *ctx, nxt_unit_r
equest_t *r)
path.length = r->path_length;
path.start = nxt_unit_sptr_get(&r->path);
+ if (((path.length > 4
+ && nxt_memcmp(path.start + (path.length - 4), ".php", 4) != 0)
+ || path.length < 5)
+ && path.start[path.length - 1] != '/'
+ && ctx->root->length + path.length < PATH_MAX)
+ {
+ char tpath[PATH_MAX];
+ struct stat sb;
+
+ 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)) {
+ nxt_php_do_err_response(ctx->req, NXT_HTTP_MOVED_PERMANENTLY);
+ return;
+ }
+ }
+
nxt_str_null(&script_name);
ctx->path_info.start = (u_char *) strstr((char *) path.start, ".php/");
diff --git a/src/nxt_router.c b/src/nxt_router.c
index f02bf3f2..56676e57 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 5dbf648d..febc599c 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_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;
+ nxt_unit_sptr_t local_port;
nxt_unit_sptr_t server_name;
nxt_unit_sptr_t target;
nxt_unit_sptr_t path;
The fix for this has been merged. a032744