diff --git a/CHANGELOG.md b/CHANGELOG.md index 76e8cdd..c36fdaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,20 @@ and the compatibility contract in [docs/STABILITY.md](docs/STABILITY.md). ## Unreleased +### Fixed + +- **Worker mode: a multipart form's `[]` and nested fields reach the app.** `$r['post']` + had each field's name as a key exactly as sent, so `tags[]` was a key of its own, a + second `tags[]` overwrote the first, and `user[name]` was never nested. Laravel saw + neither `tags` nor `user` — in any form with a file input, since those are the forms + that post multipart. `$r['post']` is now registered by PHP's own rules + (`php_register_variable_safe`), exactly as PHP builds `$_POST` from the same body — + checked against PHP itself in `a_worker_s_post_fields_are_shaped_as_php_shapes_post`, + which fails on the old code with `"tags[]":"b"`. `examples/laravel-worker.php` nests + the files the same way (`user[avatar]` reaches `$request->file('user.avatar')`; only + `name` and `name[]` worked before), and the Laravel smoke checks both. A worker script + that read raw names from `$r['post']` (`$r['post']['tags[]']`) gets PHP's shape now. + ## 1.9.0 — 2026-10-25 Worker mode now applies PHP's upload settings, as per-request mode has since 1.8.5: diff --git a/crates/askr-php/csrc/shim.c b/crates/askr-php/csrc/shim.c index 1924e43..a428735 100644 --- a/crates/askr-php/csrc/shim.c +++ b/crates/askr-php/csrc/shim.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -665,11 +666,33 @@ static PHP_FUNCTION(askr_handle_request) { add_assoc_zval(&request, "headers", &headers); add_assoc_stringl(&request, "body", w_body ? w_body : "", w_body_len); - /* Parsed multipart POST fields → $request['post']. */ + /* Parsed multipart POST fields → $request['post'], registered as PHP registers $_POST + * from a multipart body (main/rfc1867.c): no more than max_input_vars of them, and by + * PHP's own rules for names — `tags[]` appends, `user[name]` nests, a repeated name + * keeps its last value, `.` and ` ` in a top-level name become `_`. Until 1.9.1 each + * name was a key as sent, so `tags[]` was a key of its own and the second `tags[]` + * overwrote the first. + * + * Not through sapi_module.input_filter, as rfc1867.c goes: ext/filter's filter + * registers a POST variable into the real $_POST and its own filter_input() store and + * returns 0 — in a worker, one PHP request that never ends, both would grow with every + * request and carry each one's fields into the next. What that skips is filter.default, + * which PHP has deprecated. + * + * The over-limit warning goes to stderr: raised here, inside askr_handle_request(), an + * app's error handler would turn it into an exception in the middle of the worker + * loop. */ zval post; array_init(&post); + zend_long max_vars = PG(max_input_vars); for (int i = 0; i < w_npost; i++) { - add_assoc_string(&post, w_pnames[i], w_pvalues[i] ? w_pvalues[i] : ""); + const char *v = w_pvalues[i] ? w_pvalues[i] : ""; + if (i < max_vars) { + php_register_variable_safe(w_pnames[i], v, strlen(v), &post); + } else if (i == max_vars) { + fprintf(stderr, "askr: request has more than max_input_vars (" ZEND_LONG_FMT + ") POST fields; the rest are dropped, as PHP drops them\n", max_vars); + } } add_assoc_zval(&request, "post", &post); diff --git a/crates/askr/tests/e2e.rs b/crates/askr/tests/e2e.rs index a2a38a3..040942f 100644 --- a/crates/askr/tests/e2e.rs +++ b/crates/askr/tests/e2e.rs @@ -2434,6 +2434,103 @@ root = "{ROOT}" ); } +/// In worker mode `$r['post']` is what PHP's `$_POST` would be for the same multipart body: +/// `tags[]` appends, `user[name]` nests, a repeated name keeps its last value, `.` and ` ` +/// in a top-level name become `_`. +/// +/// Until 1.9.1 each name was a key as sent: `tags[]` a key of its own, the second `tags[]` +/// overwriting the first. Laravel then saw neither `tags` nor `user` in any form with a +/// file input — those are the forms that post multipart. The oracle here is PHP itself: +/// the same body, sent to a per-request server, where PHP's own parser builds `$_POST`. +/// A second request shows nothing carries over, and that the worker's own `$_POST` is +/// never written — ext/filter's input filter would write it, and keep every request's +/// fields there for the next. +#[test] +fn a_worker_s_post_fields_are_shaped_as_php_shapes_post() { + let b = "----askr-e2e-boundary"; + let field = |name: &str, value: &str| { + format!("--{b}\r\nContent-Disposition: form-data; name=\"{name}\"\r\n\r\n{value}\r\n") + }; + let body = [ + field("tags[]", "a"), + field("tags[]", "b"), + field("note", "hi"), + field("note", "again"), + field("user[name]", "kh"), + field("user[roles][]", "admin"), + field("a.b", "1"), + field("x y", "2"), + field("arr[k.l]", "3"), + format!( + "--{b}\r\nContent-Disposition: form-data; name=\"doc\"; filename=\"a.txt\"\r\n\ + Content-Type: text/plain\r\n\r\nx\r\n--{b}--\r\n" + ), + ] + .concat(); + let post = |port: u16, body: &str| { + let r = request_with_body( + port, + "POST", + "/", + &[( + "Content-Type", + &format!("multipart/form-data; boundary={b}"), + )], + body, + ); + assert_eq!(r.status, 200, "{}", r.body); + r.body.trim().to_string() + }; + + // PHP's own answer. + let config = r#" +[server] +listen = "127.0.0.1:{PORT}" +root = "{ROOT}" +"#; + let php = Server::start_in( + unique_dir("post-shape-php"), + &[("index.php", " value`, names as sent (`tags[]` is not nested, and a repeated name keeps its last value); empty for any other body, which stays in `body` | -| `files` | a `multipart/form-data` body's files, a list of `field` (the form field's name, as sent), `name` (the file's basename, as PHP's `$_FILES` gives it), `full_path` (the filename as the client sent it, path included — never use it as a path), `type` (the part's Content-Type up to any `;`, empty if it had none), `tmp_name`, `size`, `error` (`UPLOAD_ERR_*`: a file refused by `upload_max_filesize` or `MAX_FILE_SIZE` is here with its error and no `tmp_name`, `size` or `type`). PHP's upload settings apply, as in PHP: see [File uploads](FEATURES.md#8-file-uploads-040-per-request-mode-185) | +| `post` | a `multipart/form-data` body's fields, shaped as PHP shapes `$_POST` (from 1.9.1): `tags[]` appends, `user[name]` nests, a repeated name keeps its last value, `.` and ` ` in a top-level name become `_`, at most `max_input_vars` of them. Empty for any other body, which stays in `body` | +| `files` | a `multipart/form-data` body's files, a flat list of `field` (the form field's name, as sent — `photos[]`, `user[avatar]`; nest it as PHP nests `$_FILES`, as `examples/laravel-worker.php` does with `parse_str()`), `name` (the file's basename, as PHP's `$_FILES` gives it), `full_path` (the filename as the client sent it, path included — never use it as a path), `type` (the part's Content-Type up to any `;`, empty if it had none), `tmp_name`, `size`, `error` (`UPLOAD_ERR_*`: a file refused by `upload_max_filesize` or `MAX_FILE_SIZE` is here with its error and no `tmp_name`, `size` or `type`). PHP's upload settings apply, as in PHP: see [File uploads](FEATURES.md#8-file-uploads-040-per-request-mode-185) | The handler produces its response the normal PHP way — `echo`/`print` for the body, `header()` for headers, `http_response_code()` for the status — all diff --git a/examples/laravel-worker.php b/examples/laravel-worker.php index 7f23dda..2a76b75 100644 --- a/examples/laravel-worker.php +++ b/examples/laravel-worker.php @@ -90,24 +90,26 @@ parse_str($r['body'], $post); } } - $files = []; - foreach ($r['files'] ?? [] as $f) { + // Files, nested the way PHP nests $_FILES: one parse_str() of every field name, by + // PHP's own rules for brackets — `photos[]` appends, `user[avatar]` nests — with each + // file's index as its leaf, then the index swapped for the file. + $uploads = $names = []; + foreach ($r['files'] ?? [] as $i => $f) { // The name as sent, path included: Symfony keeps only its basename as the client's // name, and the whole of it for getClientOriginalPath() (folder uploads). - $uploaded = new Illuminate\Http\UploadedFile( + $uploads[$i] = new Illuminate\Http\UploadedFile( $f['tmp_name'], $f['full_path'] ?? $f['name'], $f['type'] ?: null, $f['error'] ?? 0, true // test mode ); - $field = $f['field']; - if (str_ends_with($field, '[]')) { - $files[substr($field, 0, -2)][] = $uploaded; - } else { - $files[$field] = $uploaded; - } + $names[] = rawurlencode($f['field']) . '=' . $i; } + parse_str(implode('&', $names), $files); + array_walk_recursive($files, function (&$leaf) use ($uploads) { + $leaf = $uploads[(int) $leaf]; + }); $request = Illuminate\Http\Request::create( $r['uri'], diff --git a/scripts/laravel-smoke.sh b/scripts/laravel-smoke.sh index 83f8a82..d00c899 100755 --- a/scripts/laravel-smoke.sh +++ b/scripts/laravel-smoke.sh @@ -80,6 +80,9 @@ Illuminate\Support\Facades\Route::post('/_askr_upload', fn (Illuminate\Http\Requ ? $r->file('doc')?->getClientOriginalPath() : 'n/a', 'content' => $r->file('doc')?->isValid() ? $r->file('doc')->get() : null, 'error' => $r->file('doc')?->getError(), + 'tags' => $r->input('tags'), + 'user' => $r->input('user'), + 'avatar' => $r->file('user.avatar')?->getClientOriginalName(), ]))->withoutMiddleware([ Illuminate\Foundation\Http\Middleware\PreventRequestForgery::class, Illuminate\Foundation\Http\Middleware\ValidateCsrfToken::class, @@ -213,6 +216,13 @@ n=$(echo "$j" | field name); pa=$(echo "$j" | field path) check "an upload reaches Laravel with its field and content" "$([ "$(echo "$j" | field note)" = hello ] && [ "$(echo "$j" | field content)" = "file body" ] && echo ok || echo "$j")" check "an upload's name is its basename, its path kept beside it" "$([ "$n" = a.txt ] && { [ "$pa" = photos/2024/a.txt ] || [ "$pa" = n/a ]; } && echo ok || echo "name = $n, path = $pa")" +# A form with a file input posts multipart: its checkboxes (tags[]) and nested inputs +# (user[name], user[avatar]) must reach Laravel as PHP shapes them, not as raw names. +j=$(curl -s --http1.1 -F "tags[]=a" -F "tags[]=b" -F "user[name]=kh" \ + -F "user[avatar]=@$WORK/upload.txt;filename=me.png" -F "doc=@$WORK/upload.txt" "$BASE/_askr_upload") +got=$(echo "$j" | python3 -c 'import json,sys; d=json.load(sys.stdin); print(json.dumps([d.get("tags"), d.get("user"), d.get("avatar")]))') +check "a multipart form's [] and nested names reach Laravel as PHP shapes them" "$([ "$got" = '[["a", "b"], {"name": "kh"}, "me.png"]' ] && echo ok || echo "$got")" + # PHP's upload_max_filesize holds in worker mode too (default 2M): a 3 MB file reaches # Laravel as UPLOAD_ERR_INI_SIZE, not as a file. head -c 3145728 /dev/zero > "$WORK/big.bin" diff --git a/scripts/phpt-uploads.py b/scripts/phpt-uploads.py index e03a3a9..ad9307c 100755 --- a/scripts/phpt-uploads.py +++ b/scripts/phpt-uploads.py @@ -18,7 +18,7 @@ `--POST_RAW--` is the header, the remaining lines are the body joined with "\\n", and `--INI--` is the php.ini. In per-request mode the output must match `--EXPECT--`, or `--EXPECTF--` with its placeholders. In worker mode the test script runs inside a worker -that builds `$_POST` and `$_FILES` from `$r` with PHP's own variable parser, and its +that takes `$_POST` from `$r` and builds `$_FILES` with PHP's own variable parser, and its output must match per-request mode's for the same request (see `run_one`). Known divergences are listed by name in `scripts/phpt-uploads.known` — a test there is @@ -36,8 +36,9 @@ # bracket rules (parse_str), so a difference here is a difference in what Askr parsed. WORKER = r"""