From 1c3139bb7bfd40a3b6b094f237c62488b51d6004 Mon Sep 17 00:00:00 2001 From: Panagiotis Koilakos <45639356+Svestis@users.noreply.github.com> Date: Tue, 29 Sep 2026 23:11:07 +0200 Subject: [PATCH] BE: address app.conf escaping review feedback Keep encode_python_string() in front/php/server/util.php, replacing encode_single_quotes() in place, and remove the standalone app_conf_encode.php helper file and its require. Rework the regression test to dispatch the real util.php savesettings path through the PHP CLI against temporary synthetic config, API and session directories, covering both scalar string and array string settings: the generated app.conf must compile with warnings as errors and parse back to the typed values. The existing {s-quote} lifecycle is preserved (single quotes are still written as {s-quote} and converted back by the same consumers), and the app.conf readers in initialise.py and plugin_helper.py are unchanged. --- front/php/server/app_conf_encode.php | 18 --- front/php/server/util.php | 10 +- test/backend/test_app_conf_string_escaping.py | 108 +++++++++++++----- 3 files changed, 88 insertions(+), 48 deletions(-) delete mode 100644 front/php/server/app_conf_encode.php diff --git a/front/php/server/app_conf_encode.php b/front/php/server/app_conf_encode.php deleted file mode 100644 index 1690c1ef2..000000000 --- a/front/php/server/app_conf_encode.php +++ /dev/null @@ -1,18 +0,0 @@ - "savesettings", "settings" => json_encode($in["settings"])];' + 'require $in["front"] . "/php/server/util.php";' ) +# Minimal app.conf that lets globals.php and security.php load without a password prompt. +SEED_APP_CONF = "TIMEZONE='UTC'\nSETPWD_enable_password=False\n" + CASES = { "ordinary_text": "hello world", "doc_regex": r"192\.0\.2\..*", @@ -42,19 +51,52 @@ CASES = { } +def scalar_key(case_id): + """Return the app.conf key used for the scalar string setting of a case.""" + return f"S_{case_id.upper()}" + + +def array_key(case_id): + """Return the app.conf key used for the array string setting of a case.""" + return f"A_{case_id.upper()}" + + @pytest.fixture(scope="module") -def encoded(): - """Return {case_id: encoded} produced by one PHP CLI run of encode_python_string().""" - payload = json.dumps({"path": str(ENCODER_PHP), "cases": list(CASES.values())}) +def app_conf(tmp_path_factory): + """Run util.php saveSettings() once for all cases and return the generated app.conf source.""" + root = tmp_path_factory.mktemp("app_conf") + config_dir = root / "config" + api_dir = root / "api" + session_dir = root / "session" + for folder in (config_dir, api_dir, session_dir): + folder.mkdir() + (config_dir / "app.conf").write_text(SEED_APP_CONF) + + # UI_WAIT_FOR_SETTINGS=True keeps getReloadWaitRequired() from reading the (absent) API files. + settings = [["General", "UI_WAIT_FOR_SETTINGS", "boolean", True]] + for case_id, typed in CASES.items(): + settings.append(["Test", scalar_key(case_id), "string", typed]) + settings.append(["Test", array_key(case_id), "array", ["plain", typed]]) + + env = dict(os.environ, NETALERTX_CONFIG=str(config_dir), NETALERTX_API=str(api_dir)) result = subprocess.run( - [PHP_BIN, "-r", PHP_RUNNER], - input=payload, + [PHP_BIN, "-d", f"session.save_path={session_dir}", "-d", "display_errors=stderr", "-r", PHP_RUNNER], + input=json.dumps({"front": str(FRONT_DIR), "settings": settings}), capture_output=True, text=True, + env=env, timeout=60, check=True, ) - return dict(zip(CASES.keys(), json.loads(result.stdout))) + assert json.loads(result.stdout)["success"] is True, result.stdout + result.stderr + return (config_dir / "app.conf").read_text() + + +def setting_line(source, key): + """Return the single app.conf line that assigns key.""" + lines = [line for line in source.splitlines() if line.startswith(f"{key}=")] + assert len(lines) == 1, f"expected one {key}= line, got {lines}" + return lines[0] def parse_app_conf(source): @@ -73,24 +115,32 @@ def test_warning_check_rejects_unescaped_backslash(): parse_app_conf(r"X='192\.0\.2\..*'") -@pytest.mark.parametrize("case_id", CASES.keys()) -def test_scalar_string_round_trip(encoded, case_id): - """A scalar string setting (X='') compiles cleanly and parses back to the typed value.""" - typed = CASES[case_id] - conf = parse_app_conf(f"X='{encoded[case_id]}'\n") - assert conf["X"] == typed.replace("'", "{s-quote}") +def test_generated_app_conf_compiles(app_conf): + """The whole app.conf written by saveSettings() compiles without warnings.""" + parse_app_conf(app_conf) @pytest.mark.parametrize("case_id", CASES.keys()) -def test_array_string_round_trip(encoded, case_id): - """An array setting element (X=['plain','']) compiles cleanly and parses back to the typed value.""" - typed = CASES[case_id] - conf = parse_app_conf(f"X=['plain','{encoded[case_id]}']\n") - assert conf["X"] == ["plain", typed.replace("'", "{s-quote}")] +def test_scalar_string_round_trip(app_conf, case_id): + """A scalar string setting written by saveSettings() compiles cleanly and parses back to the typed value.""" + key = scalar_key(case_id) + conf = parse_app_conf(setting_line(app_conf, key)) + assert conf[key] == CASES[case_id].replace("'", "{s-quote}") -def test_doc_regex_exact_source(encoded): +@pytest.mark.parametrize("case_id", CASES.keys()) +def test_array_string_round_trip(app_conf, case_id): + """An array string setting written by saveSettings() compiles cleanly and parses back to the typed values.""" + key = array_key(case_id) + conf = parse_app_conf(setting_line(app_conf, key)) + assert conf[key] == ["plain", CASES[case_id].replace("'", "{s-quote}")] + + +def test_doc_regex_exact_source(app_conf): """The documentation regex is emitted with doubled backslashes and parses back unchanged.""" - source = f"ICMP_IN_REGEX='{encoded['doc_regex']}'" - assert source == r"ICMP_IN_REGEX='192\\.0\\.2\\..*'" - assert parse_app_conf(source)["ICMP_IN_REGEX"] == r"192\.0\.2\..*" + scalar = setting_line(app_conf, scalar_key("doc_regex")) + array = setting_line(app_conf, array_key("doc_regex")) + assert scalar == r"S_DOC_REGEX='192\\.0\\.2\\..*'" + assert array == r"A_DOC_REGEX=['plain','192\\.0\\.2\\..*']" + assert parse_app_conf(scalar)["S_DOC_REGEX"] == r"192\.0\.2\..*" + assert parse_app_conf(array)["A_DOC_REGEX"] == ["plain", r"192\.0\.2\..*"]