From 769c59ec9e64ad8c860c36d1ba5d45f23134c9ec Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Tue, 20 Oct 2026 13:18:37 -0700 Subject: [PATCH] fix(forms): encode form action URLs and configured attributes (#8437) * fix(forms): preserve action URLs in HTML and JavaScript Signed-off-by: Thomas Vincent * docs(changelog): record form action URL fix for issue 8438 Signed-off-by: Thomas Vincent * fix(forms): encode configured form name and enctype attributes Signed-off-by: Thomas Vincent --------- Signed-off-by: Thomas Vincent Co-authored-by: TheWitness --- changelog.d/8438.issue | 1 + lib/html_form.php | 6 +- tests/Unit/Ui/Html/FormActionEncodingTest.php | 170 ++++++++++++++++++ 3 files changed, 174 insertions(+), 3 deletions(-) create mode 100644 changelog.d/8438.issue create mode 100644 tests/Unit/Ui/Html/FormActionEncodingTest.php diff --git a/changelog.d/8438.issue b/changelog.d/8438.issue new file mode 100644 index 000000000..b67fff1ab --- /dev/null +++ b/changelog.d/8438.issue @@ -0,0 +1 @@ +Preserve form action URLs and configured attributes with context-specific encoding diff --git a/lib/html_form.php b/lib/html_form.php index 30b35c490..69b633748 100644 --- a/lib/html_form.php +++ b/lib/html_form.php @@ -47,7 +47,7 @@ function draw_edit_form(array $array) : void { if (cacti_sizeof($fields_array)) { if (!isset($config_array['no_form_tag'])) { - print "
'; + print "'; } $i = 0; @@ -2180,7 +2180,7 @@ function form_start(mixed $action, string $id = '', bool $multipart = false) : v $form_action = $action; - print "'; + print "'; } /** @@ -2277,7 +2277,7 @@ function form_end(bool $ajax = true) : void { } $(function() { - formValidate('#', ''); + formValidate('#', ); }); ['graphs.php'], + 'query' => ["/cacti/graphs.php?filter=Bob's switch&rows=20#results"], + 'entities' => ['/cacti/graphs.php?filter=©"&'], + 'Unicode' => ["/cacti/graphs.php?filter=東京 \"switch\"\u{2028}next\u{2029}last"], + 'script text' => ["/cacti/graphs.php?filter=

notes

\\'"], + 'empty' => [''], + 'zero' => ['0'], +]); + +beforeEach(function () { + $this->savedFormGlobals = []; + + foreach (['form_id', 'form_action', '_SESSION'] as $name) { + $this->savedFormGlobals[$name] = [ + 'present' => array_key_exists($name, $GLOBALS), + 'value' => $GLOBALS[$name] ?? null, + ]; + } +}); + +afterEach(function () { + foreach ($this->savedFormGlobals as $name => $saved) { + if ($saved['present']) { + $GLOBALS[$name] = $saved['value']; + } else { + unset($GLOBALS[$name]); + } + } +}); + +it('preserves form action URLs in HTML and AJAX initialization', function (string $action) { + ob_start(); + + try { + form_start($action, 'edit_form', true); + form_end(); + $html = ob_get_contents(); + } finally { + ob_end_clean(); + } + + $document = new DOMDocument(); + $previous = libxml_use_internal_errors(true); + + try { + expect($document->loadHTML('' . $html))->toBeTrue(); + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($previous); + } + + $forms = $document->getElementsByTagName('form'); + expect($forms->length)->toBe(1); + $form = $forms->item(0); + expect($form->getAttribute('action'))->toBe($action) + ->and($form->getAttribute('id'))->toBe('edit_form') + ->and($form->getAttribute('name'))->toBe('edit_form') + ->and($form->getAttribute('method'))->toBe('post') + ->and($form->getAttribute('enctype'))->toBe('multipart/form-data') + ->and($GLOBALS['form_action'])->toBe($action); + + $scripts = $document->getElementsByTagName('script'); + expect($scripts->length)->toBe(1); + $script = $scripts->item(0)->textContent; + expect(preg_match('/formValidate\(\'#edit_form\', (.*)\);/', $script, $matches))->toBe(1); + expect(json_decode($matches[1], true, 512, JSON_THROW_ON_ERROR))->toBe($action) + ->and($document->getElementsByTagName('p')->length)->toBe(0); +})->with('form action URLs'); + +it('preserves configured edit form action URLs', function (string $action) { + ob_start(); + + try { + draw_edit_form([ + 'config' => ['post_to' => $action], + 'fields' => ['id' => ['method' => 'hidden', 'value' => 7]], + ]); + print '
'; + $html = ob_get_contents(); + } finally { + ob_end_clean(); + } + + $document = new DOMDocument(); + $previous = libxml_use_internal_errors(true); + + try { + expect($document->loadHTML('' . $html))->toBeTrue(); + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($previous); + } + + $forms = $document->getElementsByTagName('form'); + expect($forms->length)->toBe(1) + ->and($forms->item(0)->getAttribute('action'))->toBe($action) + ->and($forms->item(0)->getAttribute('method'))->toBe('post') + ->and($document->getElementsByTagName('input')->item(0)->getAttribute('value'))->toBe('7') + ->and($document->getElementsByTagName('script')->length)->toBe(0); +})->with('form action URLs'); + +it('preserves generated form IDs and the non-AJAX form mode', function () { + $ids = []; + + for ($i = 0; $i < 2; $i++) { + ob_start(); + + try { + form_start('edit.php'); + $ids[] = $GLOBALS['form_id']; + form_end(false); + $html = ob_get_contents(); + } finally { + ob_end_clean(); + } + + expect($html)->not->toContain('not->toContain('multipart/form-data') + ->toContain(''); + } + + expect($ids[0])->toMatch('/^form[0-9]+$/') + ->and($ids[1])->toMatch('/^form[0-9]+$/') + ->not->toBe($ids[0]); +}); + +it('keeps configured form attributes as literal values', function (string $name, string $enctype) { + ob_start(); + + try { + draw_edit_form([ + 'config' => ['post_to' => 'edit.php', 'form_name' => $name, 'enctype' => $enctype], + 'fields' => ['id' => ['method' => 'hidden', 'value' => 7]], + ]); + print ''; + $html = ob_get_contents(); + } finally { + ob_end_clean(); + } + + $document = new DOMDocument(); + expect($document->loadHTML($html))->toBeTrue(); + $form = $document->getElementsByTagName('form')->item(0); + expect($form->getAttribute('name'))->toBe($name) + ->and($form->getAttribute('enctype'))->toBe($enctype) + ->and($form->attributes->length)->toBe(6); +})->with([ + 'normal' => ['edit_form', 'multipart/form-data'], + 'name quotes and entities' => ["Bob's "form", 'multipart/form-data'], + 'enctype quoting' => ['edit_form', "multipart/form-data' data-extra='value"], +]);