fix(static): finish gitignore's rule for hide patterns

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Security
 
+- **Three shapes of hide pattern matched nothing, and nothing reads as hidden (#309).** `hide('/index.php')`, `hide('cache/')` and `hide('logs/**')` are the forms an operator carries over from gitignore; the first two covered no path at all, and the third stopped one level down, so `logs/deep/app.log` was served. A leading separator anchored the pattern and then compared it against a mount-relative path that never carries one; a trailing separator was compared literally against paths that are never bare directories; `**` met `FNM_PATHNAME`, which stops each `*` at the separator. All three fail in the direction that discloses: the pattern is accepted without complaint and the file goes on the wire. #270 gave the rule its other two shapes; this finishes it — a leading `/` pins the mount root, a trailing `/` names a directory wherever it sits along with everything under it, and `**` crosses separators for the whole pattern. A bare pattern still reads the file name, so `cache/` is how to cover a directory's contents. Underneath sat a platform split worth naming: the Windows `fnmatch` shim cast its `flags` away, so any fix that turns `FNM_PATHNAME` off works on POSIX and does nothing on Windows — the shim reads the flag now. `hide()` also refuses a pattern over 512 bytes rather than accepting one that can only match nothing. Evidence: `static/024` serves `root-secret`, `cached` and `logged` for the three paths against `dbe66f0` and hands all three to the handler here; the `StaticHide` unit table gains a row per shape.
 - **`sendFile()` and `StaticHandler` walked around `open_basedir` (#280).** The server opens response files itself rather than through PHP streams, and never consulted the interpreter's policy, so a handler could hand out any file the process could read: with `open_basedir` set, `fopen()` refused the path and `sendFile()` answered `200 OK` with its contents. `open_basedir` is the operator's boundary, and the handler is exactly the code it exists to contain — the trust `sendFile()` places in handler code is a contract with the application, not a licence to overrule the operator. All three entry points now call `php_check_open_basedir_ex()`: `sendFile()` throws, the send-file engine answers 500, and a mount rooted outside the boundary is refused at construction. Scope: the mount root is validated once, at construction, so narrowing `open_basedir` at runtime below an existing mount does not retroactively close it; `sendFile()` is checked per call. Evidence: `sendfile/007-sendfile-open-basedir` serves the running binary against `main` and answers 500 here.
 
 ### Fixed

include/static/http_static_path.h

@@ -51,11 +51,28 @@ http_static_path_resolve(const http_static_handler_t *mount, const char *request
  * Used by the index-file resolution loop. */
 bool http_static_path_join(char *buf, size_t cap, size_t *len, const char *name, size_t name_len);
 
-/* Whether one hide glob covers one mount-relative path. A pattern naming a
- * directory (`cache/*`) is anchored at the mount root and `*` stops at each
- * separator; a pattern naming none (`*.php`) covers a file of that name at any
- * depth. Takes the two strings rather than the mount so the rule can be held to
- * a table. Both must be NUL-terminated. */
+/* Longest hide pattern the matcher reads, once a leading or trailing separator
+ * is off it. StaticHandler::hide measures the pattern as written against the
+ * same number, so it refuses a little sooner than the matcher would stop. */
+#define HTTP_STATIC_HIDE_GLOB_MAX 512
+
+/* Whether one hide glob covers one mount-relative path, by gitignore's rule:
+ *
+ *   *.php        a file of that name at any depth
+ *   /index.php   that file at the mount root only
+ *   cache/x      anchored at the mount root, * stopping at each separator
+ *   cache/       a directory of that name at any depth, and all it holds
+ *   cache/**     anchored, * crossing separators
+ *
+ * A pattern opening with a double star names every directory, and the mount
+ * root is one of them.
+ *
+ * A bare pattern reads the file name, so a directory named like it does not
+ * drag its contents in — `cache/` says that instead. Case follows the
+ * platform's own filesystem: a pattern is read case-insensitively on Windows
+ * and case-sensitively elsewhere. Takes the two strings rather than the mount
+ * so the rule can be held to a table. Both must be NUL-terminated; a pattern
+ * longer than HTTP_STATIC_HIDE_GLOB_MAX covers nothing. */
 bool http_static_hide_glob_matches(const char *glob, const char *relative);
 
 /* Returns true when the relative path matches one of the mount's hide globs,

src/static/http_static_path.c

@@ -17,21 +17,24 @@
 #include <ctype.h>
 
 #ifdef PHP_WIN32
-/* Minimal glob matching used only for hide-path patterns. Handles '?'
- * (any single non-separator char) and '*' (any sequence of
- * non-separator chars). Case-insensitive on Windows. */
+/* Minimal glob matching used only for hide-path patterns. Handles '?' (any
+ * single character) and '*' (any sequence). Without FNM_PATHNAME both cross
+ * the separator, which is how a caller spells "at any depth".
+ * Case-insensitive on Windows. */
 # define FNM_PATHNAME 0x01
-static int win32_fnmatch_impl(const char *p, const char *s)
+static int win32_fnmatch_impl(const char *p, const char *s, bool cross_separator)
 {
 	while (*p) {
 		if (*p == '?') {
-			if (!*s || *s == '/') return 1;
+			if (!*s || (!cross_separator && *s == '/')) return 1;
 			p++; s++;
 		} else if (*p == '*') {
-			p++;
+			/* A run of stars says no more than one does, and each extra star
+			 * would double the positions the branch below tries. */
+			while (*p == '*') p++;
 			do {
-				if (win32_fnmatch_impl(p, s) == 0) return 0;
-				if (*s == '/' || !*s) break;
+				if (win32_fnmatch_impl(p, s, cross_separator) == 0) return 0;
+				if (!*s || (!cross_separator && *s == '/')) break;
 			} while (*s++);
 			return 1;
 		} else {
@@ -43,8 +46,7 @@ static int win32_fnmatch_impl(const char *p, const char *s)
 }
 static int fnmatch(const char *pattern, const char *string, int flags)
 {
-	(void)flags;
-	return win32_fnmatch_impl(pattern, string);
+	return win32_fnmatch_impl(pattern, string, (flags & FNM_PATHNAME) == 0);
 }
 #else
 # include <fnmatch.h>
@@ -293,30 +295,91 @@ bool http_static_path_join(char *buf, const size_t cap, size_t *len,
 	return true;
 }
 
+/* Whether the pattern names the path itself. An anchored pattern is read against
+ * the whole mount-relative path; a bare one against the file name, because the
+ * directory a file sits in is not part of what a bare pattern reads. */
+static bool http_static_hide_glob_names_path(const char *pattern, const char *relative,
+											 bool anchored, int flags)
+{
+	if (anchored) {
+		if (fnmatch(pattern, relative, flags) == 0) {
+			return true;
+		}
+
+		/* A pattern opening with a double star and a separator reads "in every
+		 * directory", and the mount root is one: there the separator has
+		 * nothing to match, so the root's own file needs the prefix off. */
+		return strncmp(pattern, "**/", 3) == 0 && fnmatch(pattern + 3, relative, flags) == 0;
+	}
+
+	const char *const basename = strrchr(relative, '/');
+
+	return fnmatch(pattern, basename != NULL ? basename + 1 : relative, flags) == 0;
+}
+
 bool http_static_hide_glob_matches(const char *glob, const char *relative)
 {
 	if (glob == NULL || relative == NULL) {
 		return false;
 	}
 
-	if (fnmatch(glob, relative, FNM_PATHNAME) == 0) {
+	/* gitignore's rule, because it is the one an operator already knows and the
+	 * one that fails safe: a separator at the front or the middle anchors the
+	 * pattern at the mount root, a separator at the end names a directory, and
+	 * a pattern with neither names a file wherever it sits. Getting the last one
+	 * wrong costs a disclosure rather than a 404 — `*.php` covering `index.php`
+	 * and handing `admin/tools.php` to the client as source. */
+	const bool rooted = (glob[0] == '/');
+	const char *const body = glob + (rooted ? 1 : 0);
+	size_t body_len = strlen(body);
+	const bool names_directory = (body_len > 0 && body[body_len - 1] == '/');
+
+	if (names_directory) {
+		body_len--;
+	}
+
+	/* StaticHandler::hide refuses a longer pattern, so this is unreachable from
+	 * the PHP API; the matcher is public and answers for itself. */
+	if (body_len == 0 || body_len > HTTP_STATIC_HIDE_GLOB_MAX) {
+		return false;
+	}
+
+	char pattern[HTTP_STATIC_HIDE_GLOB_MAX + 1];
+	memcpy(pattern, body, body_len);
+	pattern[body_len] = '\0';
+
+	const bool anchored = rooted || memchr(pattern, '/', body_len) != NULL;
+	/* `**` is how an operator spells "across directories", so the flag that
+	 * stops `*` at a separator comes off for the whole pattern. */
+	const int flags = strstr(pattern, "**") != NULL ? 0 : FNM_PATHNAME;
+
+	if (http_static_hide_glob_names_path(pattern, relative, anchored, flags)) {
 		return true;
 	}
 
-	/* A pattern that names no directory names a file, and that file is hidden
-	 * wherever it sits — gitignore's rule, and the one an operator writing
-	 * `*.php` means. FNM_PATHNAME stops `*` at the separator, so without this
-	 * the pattern covers `index.php` and serves `admin/tools.php` as source:
-	 * the surprise costs a disclosure rather than a 404. A pattern that does
-	 * name a directory stays anchored at the mount root, which is the only way
-	 * to say `cache/*` and mean that one directory. */
-	if (strchr(glob, '/') != NULL) {
+	if (!names_directory) {
 		return false;
 	}
 
-	const char *const basename = strrchr(relative, '/');
+	/* Hiding a directory hides what is under it, at any depth. Spelling "under"
+	 * as a pattern keeps the path itself free of the per-request copy that
+	 * walking its separators would need; `*` crosses them here whatever the
+	 * pattern said, because depth below a hidden directory does not matter. */
+	char under[HTTP_STATIC_HIDE_GLOB_MAX + 5];
+
+	snprintf(under, sizeof(under), "%s/*", pattern);
+
+	if (fnmatch(under, relative, 0) == 0) {
+		return true;
+	}
+
+	if (anchored) {
+		return false;
+	}
+
+	snprintf(under, sizeof(under), "*/%s/*", pattern);
 
-	return basename != NULL && fnmatch(glob, basename + 1, FNM_PATHNAME) == 0;
+	return fnmatch(under, relative, 0) == 0;
 }
 
 bool http_static_path_is_hidden(const http_static_handler_t *mount, const char *relative,

src/static/static_handler_class.c

@@ -18,6 +18,7 @@
 #include "php_http_server.h"
 #include "main/fopen_wrappers.h" /* php_check_open_basedir_ex */
 #include "static/static_handler.h"
+#include "static/http_static_path.h"  /* HTTP_STATIC_HIDE_GLOB_MAX — the bound hide() holds */
 
 #include <stdint.h>
 #include <string.h>
@@ -770,6 +771,15 @@ ZEND_METHOD(TrueAsync_StaticHandler, hide)
 								 "StaticHandler hide pattern must not be empty", 0);
 			return;
 		}
+
+		/* A pattern the matcher cannot read covers nothing, and nothing is the
+		 * answer an operator reads as "hidden". Refuse it where it is written. */
+		if (Z_STRLEN(args[i]) > HTTP_STATIC_HIDE_GLOB_MAX) {
+			zend_throw_exception_ex(http_server_invalid_argument_exception_ce, 0,
+									"StaticHandler hide pattern must be at most %d bytes",
+									HTTP_STATIC_HIDE_GLOB_MAX);
+			return;
+		}
 	}
 
 	const size_t new_count = mount->hide_count + argc;

stubs/StaticHandler.php

@@ -148,8 +148,22 @@ public function setSymlinkPolicy(StaticSymlinks $policy): static {}
     /**
      * Glob patterns whose matching paths return 404 regardless of
      * existence. Patterns are matched against the path *relative to
-     * the root directory*, with `/` as the separator.
+     * the root directory*, with `/` as the separator, by gitignore's rule:
      *
+     *   `*.php`      a file of that name at any depth
+     *   `/index.php` that file at the root directory only
+     *   `cache/x`    anchored at the root directory, `*` stopping at each `/`
+     *   `cache/`     a directory of that name at any depth, and all it holds
+     *   `cache/**`   anchored, `*` crossing `/`
+     *
+     * A pattern opening with a double star names every directory, the root
+     * directory among them. A pattern without `/` reads the file name, so a
+     * directory named like it keeps serving what it holds — `cache/` is how to
+     * cover a directory. Case follows the platform's own filesystem:
+     * case-insensitive on Windows, case-sensitive elsewhere.
+     *
+     * @throws HttpServerInvalidArgumentException when a pattern is empty or
+     *         longer than 512 bytes.
      * @return static
      */
     public function hide(string ...$globs): static {}

tests/phpt/server/static/024-static-hide-gitignore-forms.phpt

@@ -0,0 +1,126 @@
+--TEST--
+StaticHandler::hide(): a leading separator pins the root, a trailing one names a directory, ** crosses
+--EXTENSIONS--
+true_async_server
+true_async
+--FILE--
+<?php
+/* The three forms an operator carries over from gitignore. Each of them used to
+ * match nothing at all, which reads exactly like "hidden" until the file comes
+ * back on the wire — the same disclosure #270 was about, reached by writing a
+ * pattern in a shape the matcher did not read.
+ *
+ * /index.php names the root's own file and leaves its namesakes alone, cache/
+ * names a directory wherever it sits, logs/** crosses separators the way logs/*
+ * does not, and a pattern opening with a double star reaches the root as well
+ * as everything under it. */
+
+use TrueAsync\HttpServer;
+use TrueAsync\HttpServerConfig;
+use TrueAsync\StaticHandler;
+use TrueAsync\StaticOnMissing;
+use function Async\spawn;
+use function Async\delay;
+
+require_once __DIR__ . '/../_free_port.inc';
+
+$root = sys_get_temp_dir() . '/php-http-024-root-' . getmypid();
+@mkdir($root . '/sub', 0700, true);
+@mkdir($root . '/var/cache', 0700, true);
+@mkdir($root . '/logs/deep', 0700, true);
+
+$files = [
+    '/index.php'          => 'root-secret',
+    '/sub/index.php'      => 'nested-copy',
+    '/var/cache/x.txt'    => 'cached',
+    '/logs/deep/app.log'  => 'logged',
+    '/secret.txt'         => 'root-note',
+    '/sub/secret.txt'     => 'nested-note',
+    '/app.svg'            => 'svg-bytes',
+];
+
+foreach ($files as $path => $body) {
+    file_put_contents($root . $path, $body);
+}
+
+register_shutdown_function(function () use ($root, $files) {
+    foreach (array_keys($files) as $path) {
+        @unlink($root . $path);
+    }
+    foreach (['/sub', '/var/cache', '/var', '/logs/deep', '/logs'] as $dir) {
+        @rmdir($root . $dir);
+    }
+    @rmdir($root);
+});
+
+$port = tas_free_port();
+$server = new HttpServer(
+    (new HttpServerConfig())->addListener('127.0.0.1', $port)->setReadTimeout(5)
+);
+
+$mount = new StaticHandler('/', $root);
+$mount->setOnMissing(StaticOnMissing::NEXT);
+$mount->hide('/index.php', 'cache/', 'logs/**', '**/secret.txt');
+$server->addStaticHandler($mount);
+
+$server->addHttpHandler(function ($req, $res) {
+    $res->end('handler:' . $req->getPath());
+});
+
+spawn(function () use ($port, $server) {
+    delay(50);
+
+    foreach (array_keys($files = [
+        '/app.svg'           => null,
+        '/index.php'         => null,
+        '/sub/index.php'     => null,
+        '/var/cache/x.txt'   => null,
+        '/logs/deep/app.log' => null,
+        '/secret.txt'        => null,
+        '/sub/secret.txt'    => null,
+    ]) as $path) {
+        $c = stream_socket_client("tcp://127.0.0.1:$port", $e1, $e2, 3);
+        stream_set_timeout($c, 3);
+        fwrite($c, "GET $path HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n");
+        $raw = stream_get_contents($c);
+        fclose($c);
+
+        $body = substr($raw, strpos($raw, "\r\n\r\n") + 4);
+        echo str_pad($path, 20), '-> ', trim($body), "\n";
+    }
+
+    /* A pattern the matcher cannot read would cover nothing, and nothing is
+     * what an operator reads as hidden — hide() refuses it instead. */
+    $spare = new StaticHandler('/spare/', sys_get_temp_dir());
+
+    try {
+        $spare->hide(str_repeat('a', 512));
+        echo "512 bytes: accepted\n";
+    } catch (\Throwable $e) {
+        echo "512 bytes: ", get_class($e), "\n";
+    }
+
+    try {
+        $spare->hide(str_repeat('a', 513));
+        echo "513 bytes: accepted\n";
+    } catch (\Throwable $e) {
+        echo "513 bytes: ", get_class($e), ' - ', $e->getMessage(), "\n";
+    }
+
+    $server->stop();
+});
+
+$server->start();
+echo "Done\n";
+?>
+--EXPECT--
+/app.svg            -> svg-bytes
+/index.php          -> handler:/index.php
+/sub/index.php      -> nested-copy
+/var/cache/x.txt    -> handler:/var/cache/x.txt
+/logs/deep/app.log  -> handler:/logs/deep/app.log
+/secret.txt         -> handler:/secret.txt
+/sub/secret.txt     -> handler:/sub/secret.txt
+512 bytes: accepted
+513 bytes: TrueAsync\HttpServerInvalidArgumentException - StaticHandler hide pattern must be at most 512 bytes
+Done