From 9fec8799ac09a87fd6f3de078605719f281c6a99 Mon Sep 17 00:00:00 2001 From: Alex Hope-O'Connor Date: Wed, 9 Sep 2026 14:55:03 +1000 Subject: [PATCH] test: cover custom parameter portal regressions --- docs/TESTING.md | 30 ++++- test/portal-harness/src/main.cpp | 51 +++++++- test/test_wifimanager/test_main.cpp | 1 + test/test_wifimanager/test_main.h | 1 + .../tests/test_api_json_and_captive.cpp | 18 +++ tests/portal-contract/compose.yaml | 2 + .../portal-contract/tests/ap-contract.spec.js | 109 ++++++++++++++++-- tools/portal-hardware | 19 ++- tools/tests/test-portal-hardware-cli.sh | 11 ++ 9 files changed, 223 insertions(+), 19 deletions(-) diff --git a/docs/TESTING.md b/docs/TESTING.md index 426604d..d56aed0 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -67,18 +67,36 @@ replaced: ./tools/portal-hardware run ... --take-over-client-adapter ``` -The fixture opens a 15-minute portal session, includes one harmless custom -parameter, and verifies root/bootstrap/info/status API responses, concurrent -low-priority requests, parameter persistence, timeout reset, an actual async -scan, a missing-route response, and desktop/mobile portal rendering with no -browser page errors. Screenshots, traces on failure, JSON results, and the HTML -report are saved under the printed XDG state-directory artifact path. +The fixture opens a 15-minute portal session and includes thirteen harmless +custom parameters. It verifies root/bootstrap/info/status API responses, +concurrent low-priority requests, every custom field and value across repeated +API fetches, timeout reset, an actual async scan, a missing-route response, and +desktop/mobile portal rendering with no browser page errors. It also round-trips +a value containing apostrophes, quotes, backslashes, angle brackets, and an +ampersand through the rendered form and parameter-save API. Screenshots, traces +on failure, JSON results, and the HTML report are saved under the printed XDG +state-directory artifact path. On ESP8266, an AP+STA scan can briefly move the radio off the AP channel. The client may reconnect during that interval; the contract deliberately retries that transport interruption and still requires a reachable portal with a complete, valid scan result. +The normal browser contract catches the common regression case. When changing +parameter rendering, run the opt-in ESP8266 soak as well. It performs twelve +full browser renders and API fetches while the AP is active, asserting all +thirteen fields and their exact values on every pass. This targets the +memory-sensitive rendering failure reported upstream in issue #1787 without +making every ordinary hardware run unnecessarily long: + +```bash +./tools/portal-hardware run \ + --platform esp8266 \ + --port /dev/serial/by-id/usb-... \ + --client-interface wlx74da385d4165 \ + --custom-parameter-stress +``` + For interactive diagnosis, leave the temporary client connection up and remove only that managed connection when finished: diff --git a/test/portal-harness/src/main.cpp b/test/portal-harness/src/main.cpp index 8f1b627..f6797ff 100644 --- a/test/portal-harness/src/main.cpp +++ b/test/portal-harness/src/main.cpp @@ -13,6 +13,49 @@ constexpr char kPortalPassword[] = "default1"; WiFiManager wifi; WiFiManagerParameter kInstallationLabel( "installation_label", "Installation label", "Contract fixture", 32); +// Keep the characters from upstream issue #1863 in the portal fixture. The +// browser contract verifies this value through JSON, DOM rendering, save, and +// a subsequent reload rather than relying on a string-only serializer check. +WiFiManagerParameter kEscapedValue( + "escaped_value", "Escaped value", "7(f+4]2y3fsYTQt'Uhxc\"d\\<>&", 64); +WiFiManagerParameter kMqttHost( + "mqtt_host", "MQTT host", "broker.example.local", 64); +WiFiManagerParameter kMqttPort( + "mqtt_port", "MQTT port", "1883", 8); +WiFiManagerParameter kDeviceRoom( + "device_room", "Device room", "Workshop", 32); +WiFiManagerParameter kSensorName( + "sensor_name", "Sensor name", "Ambient temperature", 48); +WiFiManagerParameter kTelemetryTopic( + "telemetry_topic", "Telemetry topic", "sensors/ambient/temperature", 64); +WiFiManagerParameter kTimezone( + "timezone", "Timezone", "Australia/Brisbane", 48); +WiFiManagerParameter kLatitude( + "latitude", "Latitude", "-27.4698", 16); +WiFiManagerParameter kLongitude( + "longitude", "Longitude", "153.0251", 16); +WiFiManagerParameter kFirmwareChannel( + "firmware_channel", "Firmware channel", "stable", 16); +WiFiManagerParameter kOwnerName( + "owner_name", "Owner name", "Portal contract", 48); +WiFiManagerParameter kNotes( + "notes", "Notes", "Thirteen-field rendering fixture", 64); + +WiFiManagerParameter* const kPortalParameters[] = { + &kInstallationLabel, + &kEscapedValue, + &kMqttHost, + &kMqttPort, + &kDeviceRoom, + &kSensorName, + &kTelemetryTopic, + &kTimezone, + &kLatitude, + &kLongitude, + &kFirmwareChannel, + &kOwnerName, + &kNotes, +}; } // namespace @@ -32,7 +75,13 @@ void setup() { IPAddress(192, 168, 4, 1), IPAddress(192, 168, 4, 1), IPAddress(255, 255, 255, 0)); - wifi.portalAddParameter(&kInstallationLabel); + // Keep custom parameters on their own native Save parameters page. This + // lets the browser contract exercise a parameter-only submit without + // starting a station connection as part of the regression test. + wifi.portalSetLayoutParamsLocation(PortalParamsLocation::SetupPage); + for (auto* parameter : kPortalParameters) { + wifi.portalAddParameter(parameter); + } wifi.startConfigPortal(kPortalSsid, kPortalPassword); } diff --git a/test/test_wifimanager/test_main.cpp b/test/test_wifimanager/test_main.cpp index 5ff6d45..fb5d185 100644 --- a/test/test_wifimanager/test_main.cpp +++ b/test/test_wifimanager/test_main.cpp @@ -17,6 +17,7 @@ TestCase tests[] = { TEST_ENTRY(test_profile_portal_candidate_does_not_take_legacy_empty_ssid_path), TEST_ENTRY(test_api_info_json_shape), TEST_ENTRY(test_api_params_json_shape), + TEST_ENTRY(test_api_params_json_escapes_custom_parameter_value), TEST_ENTRY(test_api_status_json_shape), // Configuration tests diff --git a/test/test_wifimanager/test_main.h b/test/test_wifimanager/test_main.h index 19b8d93..65cb99c 100644 --- a/test/test_wifimanager/test_main.h +++ b/test/test_wifimanager/test_main.h @@ -123,6 +123,7 @@ void test_profile_portal_success_keeps_handoff_alive(); void test_profile_portal_candidate_does_not_take_legacy_empty_ssid_path(); void test_api_info_json_shape(); void test_api_params_json_shape(); +void test_api_params_json_escapes_custom_parameter_value(); void test_api_status_json_shape(); // State transition tests diff --git a/test/test_wifimanager/tests/test_api_json_and_captive.cpp b/test/test_wifimanager/tests/test_api_json_and_captive.cpp index 4a5d964..dad486b 100644 --- a/test/test_wifimanager/tests/test_api_json_and_captive.cpp +++ b/test/test_wifimanager/tests/test_api_json_and_captive.cpp @@ -167,6 +167,24 @@ void test_api_params_json_shape() { Serial.println("[TEST] API params JSON shape test completed successfully"); } +void test_api_params_json_escapes_custom_parameter_value() { + Serial.println("[TEST] Testing escaped custom parameter JSON value..."); + + constexpr char value[] = "7(f+4]2y3fsYTQt'Uhxc\"d\\<>&"; + WiFiManager wm; + WiFiManagerHandlers handlers(&wm); + WiFiManagerParameter field("escaped_value", "Escaped value", value, + static_cast(sizeof(value) - 1)); + wm.portalAddParameter(&field); + + const String json = handlers.buildApiParamsGetJson(); + // JSON must retain apostrophes, angle brackets and ampersands as data, + // while escaping the quote and backslash that delimit a JSON string. + TEST_ASSERT_NOT_NULL(strstr(json.c_str(), "7(f+4]2y3fsYTQt'Uhxc\\\"d\\\\<>&")); + + Serial.println("[TEST] Escaped custom parameter JSON value test completed successfully"); +} + void test_api_status_json_shape() { Serial.println("[TEST] Testing /api/status JSON shape..."); diff --git a/tests/portal-contract/compose.yaml b/tests/portal-contract/compose.yaml index 6cd2787..1d35666 100644 --- a/tests/portal-contract/compose.yaml +++ b/tests/portal-contract/compose.yaml @@ -14,6 +14,8 @@ services: HOME: /tmp PORTAL_URL: http://192.168.4.1 PORTAL_BROWSER_MODE: ${PORTAL_BROWSER_MODE:-auto} + PORTAL_CUSTOM_PARAMETER_STRESS: ${PORTAL_CUSTOM_PARAMETER_STRESS:-0} + PORTAL_PLATFORM: ${PORTAL_PLATFORM:-} PORTAL_CAPTURE_README_MEDIA: ${PORTAL_CAPTURE_README_MEDIA:-0} ARTIFACT_DIR: /artifacts volumes: diff --git a/tests/portal-contract/tests/ap-contract.spec.js b/tests/portal-contract/tests/ap-contract.spec.js index aaeee3a..b6da103 100644 --- a/tests/portal-contract/tests/ap-contract.spec.js +++ b/tests/portal-contract/tests/ap-contract.spec.js @@ -1,9 +1,35 @@ const { test, expect } = require('@playwright/test'); +const fixtureParameters = [ + { id: 'installation_label', value: 'Contract fixture' }, + { id: 'escaped_value', value: "7(f+4]2y3fsYTQt'Uhxc\"d\\<>&" }, + { id: 'mqtt_host', value: 'broker.example.local' }, + { id: 'mqtt_port', value: '1883' }, + { id: 'device_room', value: 'Workshop' }, + { id: 'sensor_name', value: 'Ambient temperature' }, + { id: 'telemetry_topic', value: 'sensors/ambient/temperature' }, + { id: 'timezone', value: 'Australia/Brisbane' }, + { id: 'latitude', value: '-27.4698' }, + { id: 'longitude', value: '153.0251' }, + { id: 'firmware_channel', value: 'stable' }, + { id: 'owner_name', value: 'Portal contract' }, + { id: 'notes', value: 'Thirteen-field rendering fixture' }, +]; +const escapedUpdatedValue = "updated 'quote\" slash\\<>&"; + async function json(response) { return JSON.parse(await response.text()); } +function expectFixtureParameters(payload, expected = fixtureParameters) { + expect(payload.params).toHaveLength(expected.length); + const byId = new Map(payload.params.map((field) => [field.id, field])); + for (const { id, value } of expected) { + expect(byId.get(id), `missing fixture field ${id}`).toBeDefined(); + expect(byId.get(id).value, `unexpected value for ${id}`).toBe(value); + } +} + async function waitForScan(request) { let result; await expect.poll(async () => { @@ -24,7 +50,7 @@ async function waitForScan(request) { } test.describe('portal AP contract', () => { - test('serves API, persists fixture parameters, and completes a real scan', async ({ request }) => { + test('serves API, retains all thirteen fixture parameters, and completes a real scan', async ({ request }) => { const root = await request.get('/'); expect(root.ok()).toBeTruthy(); expect(await root.text()).toContain(' { const bootstrap = await json(bootstrapResponse); expect(bootstrap.contractVersion).toBe(3); expect(bootstrap.context.portalActive).toBe(true); + expect(bootstrap.layout.paramsLocation).toBe('setup'); const metaResponse = await request.get('/api/wifi/meta'); expect(metaResponse.ok()).toBeTruthy(); const meta = await json(metaResponse); - expect(JSON.stringify(meta)).toContain('installation_label'); + // The fixture intentionally uses a separate Settings page. Its custom + // parameters are therefore served by /api/params rather than duplicated + // in the Wi-Fi form metadata. + expect(meta.params).toHaveLength(0); const infoResponse = await request.get('/api/info'); expect(infoResponse.ok()).toBeTruthy(); @@ -50,15 +80,14 @@ test.describe('portal AP contract', () => { const statusResponse = await request.get('/api/status'); expect(statusResponse.ok()).toBeTruthy(); - const saveResponse = await request.post('/api/params/save', { - form: { installation_label: 'Contract verified' }, - }); - expect(saveResponse.ok()).toBeTruthy(); - expect(JSON.stringify(await json(saveResponse))).toContain('saved'); - - const paramsResponse = await request.get('/api/params'); - expect(paramsResponse.ok()).toBeTruthy(); - expect(JSON.stringify(await json(paramsResponse))).toContain('Contract verified'); + // Issue #1787 was intermittent and memory-sensitive on ESP8266. Fetch + // the complete API response repeatedly so ordinary portal runs verify + // every field and value without opting into the longer browser soak. + for (let attempt = 0; attempt < 4; attempt += 1) { + const paramsResponse = await request.get('/api/params'); + expect(paramsResponse.ok(), `parameter fetch ${attempt + 1}`).toBeTruthy(); + expectFixtureParameters(await json(paramsResponse)); + } const resetResponse = await request.post('/api/portal/timeout-reset'); expect(resetResponse.ok()).toBeTruthy(); @@ -90,7 +119,10 @@ test.describe('portal AP contract', () => { await page.goto('/#/wifi', { waitUntil: 'networkidle' }); await expect(page.locator('#wm-refresh-scan')).toBeVisible(); + await page.goto('/#/setup', { waitUntil: 'networkidle' }); + await expect(page.locator('#wm-param-form input')).toHaveCount(fixtureParameters.length); await expect(page.locator('#wm-f-installation_label')).toBeVisible(); + await expect(page.locator('#wm-f-escaped_value')).toHaveValue(fixtureParameters[1].value); await page.locator('#wm-f-installation_label').fill('Browser verified'); await page.screenshot({ path: `${process.env.ARTIFACT_DIR}/portal-wifi-desktop.png`, fullPage: true }); @@ -108,4 +140,59 @@ test.describe('portal AP contract', () => { await desktop.close(); expect(errors).toEqual([]); }); + + test('round-trips quotes, apostrophes, backslashes, and HTML-sensitive values', async ({ page, request }) => { + test.skip(process.env.PORTAL_BROWSER_MODE === 'skip', 'Browser checks were explicitly skipped.'); + + await page.goto('/#/setup', { waitUntil: 'networkidle' }); + await expect(page.locator('#wm-param-form input')).toHaveCount(fixtureParameters.length); + const escapedInput = page.locator('#wm-f-escaped_value'); + await expect(escapedInput).toHaveValue(fixtureParameters[1].value); + + await escapedInput.fill(escapedUpdatedValue); + const saveResponse = page.waitForResponse((response) => ( + response.url().endsWith('/api/params/save') + && response.request().method() === 'POST' + )); + await page.locator('#wm-param-form button[type="submit"]').click(); + expect((await saveResponse).ok()).toBeTruthy(); + + const paramsResponse = await request.get('/api/params'); + expect(paramsResponse.ok()).toBeTruthy(); + const expected = fixtureParameters.map((field) => ( + field.id === 'escaped_value' ? { ...field, value: escapedUpdatedValue } : field + )); + expectFixtureParameters(await json(paramsResponse), expected); + + await page.reload({ waitUntil: 'networkidle' }); + await expect(page.locator('#wm-f-escaped_value')).toHaveValue(escapedUpdatedValue); + }); + + test('ESP8266 repeatedly renders all thirteen custom parameters', async ({ page, request }) => { + test.skip(process.env.PORTAL_CUSTOM_PARAMETER_STRESS !== '1', + 'ESP8266 custom-parameter browser stress was not requested.'); + test.skip(process.env.PORTAL_PLATFORM !== 'esp8266', + 'Custom-parameter browser stress is scoped to ESP8266.'); + + const expected = fixtureParameters.map((field) => ( + field.id === 'escaped_value' ? { ...field, value: escapedUpdatedValue } : field + )); + const resetResponse = await request.post('/api/params/save', { + form: Object.fromEntries(expected.map(({ id, value }) => [id, value])), + }); + expect(resetResponse.ok()).toBeTruthy(); + + for (let attempt = 0; attempt < 12; attempt += 1) { + const paramsResponse = await request.get('/api/params'); + expect(paramsResponse.ok(), `API fetch ${attempt + 1}`).toBeTruthy(); + const params = await json(paramsResponse); + expectFixtureParameters(params, expected); + + await page.goto('/#/setup', { waitUntil: 'networkidle' }); + await expect(page.locator('#wm-param-form input')).toHaveCount(expected.length); + for (const { id, value } of expected) { + await expect(page.locator(`#wm-f-${id}`), `${id}, render ${attempt + 1}`).toHaveValue(value); + } + } + }); }); diff --git a/tools/portal-hardware b/tools/portal-hardware index 8f4ab81..a28172f 100755 --- a/tools/portal-hardware +++ b/tools/portal-hardware @@ -14,7 +14,7 @@ Usage: ./tools/portal-hardware run --platform esp8266|esp32 --port /dev/serial/by-id/... \ --client-interface IFACE [--take-over-client-adapter] [--keep] \ [--browser auto|skip] [--station-env PATH] [--output DIRECTORY] \ - [--capture-readme-media] + [--capture-readme-media] [--custom-parameter-stress] ./tools/portal-hardware down Only the named client interface may be disconnected or reconfigured. The tool @@ -37,6 +37,7 @@ browser="auto" station_env="" output_dir="" capture_readme_media="no" +custom_parameter_stress="no" while [[ $# -gt 0 ]]; do case "$1" in --platform) [[ $# -ge 2 ]] || usage; platform="$2"; shift 2 ;; @@ -48,6 +49,7 @@ while [[ $# -gt 0 ]]; do --station-env) [[ $# -ge 2 ]] || usage; station_env="$2"; shift 2 ;; --output) [[ $# -ge 2 ]] || usage; output_dir="$2"; shift 2 ;; --capture-readme-media) capture_readme_media="yes"; shift ;; + --custom-parameter-stress) custom_parameter_stress="yes"; shift ;; *) usage ;; esac done @@ -95,6 +97,16 @@ validate_run_arguments() { exit 2 } fi + if [[ "$custom_parameter_stress" == "yes" ]]; then + [[ "$command_name" == "run" && "$platform" == "esp8266" ]] || { + echo "--custom-parameter-stress is supported only by run --platform esp8266" >&2 + exit 2 + } + [[ "$browser" == "auto" ]] || { + echo "--custom-parameter-stress requires --browser auto" >&2 + exit 2 + } + fi } write_readme_media_manifest() { @@ -196,6 +208,11 @@ case "$command_name" in export LOCAL_UID="$(id -u)" export LOCAL_GID="$(id -g)" export PORTAL_BROWSER_MODE="$browser" + export PORTAL_CUSTOM_PARAMETER_STRESS=0 + if [[ "$custom_parameter_stress" == "yes" ]]; then + export PORTAL_CUSTOM_PARAMETER_STRESS=1 + fi + export PORTAL_PLATFORM="$platform" if [[ "$capture_readme_media" == "yes" ]]; then export PORTAL_CAPTURE_README_MEDIA=1 export PORTAL_CONTRACT_DOCKER_TARGET=media diff --git a/tools/tests/test-portal-hardware-cli.sh b/tools/tests/test-portal-hardware-cli.sh index 8715b85..645b239 100755 --- a/tools/tests/test-portal-hardware-cli.sh +++ b/tools/tests/test-portal-hardware-cli.sh @@ -51,6 +51,16 @@ if "$root/tools/portal-hardware" run --platform esp8266 --port /dev/null \ echo 'ESP8266 README media capture was accepted' >&2 exit 1 fi +if "$root/tools/portal-hardware" run --platform esp32 --port /dev/null \ + --client-interface wlan-client --custom-parameter-stress >/dev/null 2>&1; then + echo 'ESP32 custom-parameter stress was accepted' >&2 + exit 1 +fi +if "$root/tools/portal-hardware" run --platform esp8266 --port /dev/null \ + --client-interface wlan-client --browser skip --custom-parameter-stress >/dev/null 2>&1; then + echo 'browser-skipped custom-parameter stress was accepted' >&2 + exit 1 +fi # A failed association must delete the only connection it just created. source "$root/tools/lib/portal-hardware-session.sh" @@ -91,6 +101,7 @@ run_line="$(grep -n " run --rm portal-contract$" "$CALL_LOG" | tail -1 | cut -d: } grep -Fq 'wait_for_portal_ready' "$root/tools/portal-hardware" grep -Fq 'api/wifi/scan-status' "$root/tools/portal-hardware" +grep -Fq 'PORTAL_CUSTOM_PARAMETER_STRESS' "$root/tools/portal-hardware" grep -Fq 'README media GIF exceeds its 2 MiB documentation budget' \ "$root/tests/portal-contract/render-readme-media.sh"