wip 1

Author: seriouscoder43

webshotd/config/static_config.yaml

@@ -44,7 +44,8 @@ components_manager:
           .yml: application/yaml
           __default__: text/plain
     config:
-      url_bytes_max: 32768
+      url_bytes_max: $url_bytes_max
+      url_bytes_max#fallback: 32768
       allowlist_only: $allowlist_only
       https_only: $https_only
       state_dir: $state_dir

webshotd/src/browser_probe_handler.cpp

@@ -349,6 +349,10 @@ void CleanupProbeSession(
 [[nodiscard]] Expected<dto::BrowserProbeResponse, String>
 RunProbe(const dto::BrowserProbeRequest &request, const ProbeConfig &config, eng::Deadline deadline)
 {
+    // Fail fast before any browser/CDP work: the request timeout can be very small in tests.
+    if (usize{request.url.size()} > config.svc_config.UrlBytesMax())
+        return Unex{"url too long"_t};
+
     auto browser = TRY(
         crawler::BrowserSession::Make(
             config.dns_resolver_, config.process_starter_, config.fs_task_processor_,
@@ -398,8 +402,6 @@ RunProbe(const dto::BrowserProbeRequest &request, const ProbeConfig &config, eng
 
         browser->MarkPhase("navigate");
         dto::PageNavigateParams navigate_params;
-        if (usize{request.url.size()} > config.svc_config.UrlBytesMax())
-            return Unex{"url too long"_t};
         navigate_params.url = request.url;
         const auto navigate_result = TRY_MAP_ERR(
             cdp_session.Send<dto::PageNavigateResult>("Page.navigate"_t, navigate_params),

webshotd/test/allowlist_handler.py

@@ -1,11 +1,8 @@
 import pytest
+from helper.config_hooks import enable_allowlist_only
 from helper.constants import TEST_ASSET_HOST, TEST_HOST
 
 
-def _enable_allowlist_only(_config_yaml, config_vars):
-    config_vars["allowlist_only"] = True
-
-
 @pytest.mark.asyncio
 async def test_allowlist_check_not_exposed_on_main_listener(service_client):
     resp = await service_client.post("/v1/allowlist/check", json={"link": "http://example.com/"})
@@ -87,7 +84,7 @@ async def test_regular_mode_allowlist_overrides_denylist(service_client, monitor
     assert resp.status == 202
 
 
-@pytest.mark.uservice_oneshot(config_hooks=[_enable_allowlist_only])
+@pytest.mark.uservice_oneshot(config_hooks=[enable_allowlist_only])
 @pytest.mark.asyncio
 async def test_allowlist_only_blocks_non_allowlisted_seed(service_client):
     resp = await service_client.post(
@@ -98,7 +95,7 @@ async def test_allowlist_only_blocks_non_allowlisted_seed(service_client):
     assert resp.json()["error"]["message"] == "link not in allowlist"
 
 
-@pytest.mark.uservice_oneshot(config_hooks=[_enable_allowlist_only])
+@pytest.mark.uservice_oneshot(config_hooks=[enable_allowlist_only])
 @pytest.mark.asyncio
 async def test_allowlist_only_denylist_wins(service_client, monitor_client):
     link = f"https://{TEST_HOST}/allowlist-only-denylist-wins"

webshotd/test/capture_flow.py

@@ -14,16 +14,16 @@
     _wacz_entries,
     _wacz_zip_compress_type,
 )
+from helper.config_hooks import set_url_bytes_max
 from helper.constants import TEST_HOST
 from helper.prefix import prefix_key_from_link
 from helper.waiters import wait_for_job_status, wait_for_purge
 
 _CDP_URL_BYTES_MAX_SMALL = 64
 
 
-def _patch_url_bytes_max_small(config_yaml, _config_vars):
-    components = config_yaml["components_manager"]["components"]
-    components["config"]["url_bytes_max"] = _CDP_URL_BYTES_MAX_SMALL
+def _patch_url_bytes_max_small(config_yaml, config_vars):
+    set_url_bytes_max(config_yaml, config_vars, _CDP_URL_BYTES_MAX_SMALL)
 
 
 @pytest.mark.asyncio
@@ -277,7 +277,8 @@ async def test_capture_records_main_document_redirect_in_wacz(
 
 @pytest.mark.uservice_oneshot(config_hooks=[_patch_url_bytes_max_small])
 @pytest.mark.asyncio
-async def test_cdp_ingest_caps_url_bytes_max(monitor_client):
+async def test_cdp_ingest_caps_url_bytes_max(service_client, monitor_client):
+    del service_client
     base = "http://test-target/?"
     over = base + ("a" * (_CDP_URL_BYTES_MAX_SMALL - len(base) + 1))
 

webshotd/test/capture_flow_https.py

@@ -2,19 +2,18 @@
 from helper.capture_flow import (
     _assert_missing_job_fields,
     _capture_and_wait,
-    _enable_allowlist_only,
-    _enable_https_only,
     _probe_replay,
     _wacz_archive_text,
     _wacz_cdx_statuses_for_root_url,
     _wacz_cdx_statuses_for_url,
     _wacz_entries,
 )
+from helper.config_hooks import enable_allowlist_only, enable_https_only
 from helper.constants import TEST_HOST, UNTRUSTED_TEST_HOST
 from helper.waiters import wait_for_job_status
 
 
-@pytest.mark.uservice_oneshot(config_hooks=[_enable_allowlist_only])
+@pytest.mark.uservice_oneshot(config_hooks=[enable_allowlist_only])
 @pytest.mark.asyncio
 async def test_allowlist_only_blocks_non_allowlisted_redirect_target(
     service_client, monitor_client
@@ -55,7 +54,7 @@ async def test_https_first_succeeds_when_http_fails(
     assert not _wacz_cdx_statuses_for_url(wacz, f"http://{TEST_HOST}/https-first-http-fails")
 
 
-@pytest.mark.uservice_oneshot(config_hooks=[_enable_https_only])
+@pytest.mark.uservice_oneshot(config_hooks=[enable_https_only])
 @pytest.mark.asyncio
 async def test_https_only_accepts_http_seed_and_crawls_https(
     service_client, browser_probe, download_wacz, service_baseurl
@@ -94,7 +93,7 @@ async def test_https_first_falls_back_to_http_when_https_no_response(
     assert not _wacz_cdx_statuses_for_url(wacz, f"https://{TEST_HOST}/http-fallback-success")
 
 
-@pytest.mark.uservice_oneshot(config_hooks=[_enable_https_only])
+@pytest.mark.uservice_oneshot(config_hooks=[enable_https_only])
 @pytest.mark.asyncio
 async def test_https_only_does_not_fall_back_to_http(service_client):
     link = f"http://{TEST_HOST}/http-fallback-success"

webshotd/test/capture_flow_subresources.py

@@ -1,12 +1,11 @@
 import pytest
 from helper.capture_flow import (
     _capture_and_wait,
-    _enable_allowlist_only,
-    _enable_https_only,
     _probe_replay,
     _wacz_archive_text,
     _wacz_cdx_statuses_for_url,
 )
+from helper.config_hooks import enable_allowlist_only, enable_https_only
 from helper.constants import TEST_ASSET_HOST, TEST_HOST
 from helper.waiters import wait_for_job_status
 
@@ -101,7 +100,7 @@ async def test_regular_mode_allowlist_overrides_denylisted_subresource_fetch(
     assert any("denylist" in entry for entry in replay["console"])
 
 
-@pytest.mark.uservice_oneshot(config_hooks=[_enable_allowlist_only])
+@pytest.mark.uservice_oneshot(config_hooks=[enable_allowlist_only])
 @pytest.mark.asyncio
 async def test_allowlist_only_fetches_allowlisted_subresources(
     service_client, monitor_client, browser_probe, download_wacz, service_baseurl
@@ -123,7 +122,7 @@ async def test_allowlist_only_fetches_allowlisted_subresources(
     assert 200 in _wacz_cdx_statuses_for_url(wacz, script)
 
 
-@pytest.mark.uservice_oneshot(config_hooks=[_enable_allowlist_only])
+@pytest.mark.uservice_oneshot(config_hooks=[enable_allowlist_only])
 @pytest.mark.asyncio
 async def test_allowlist_only_blocks_non_allowlisted_subresources(
     service_client, monitor_client, browser_probe, download_wacz, service_baseurl
@@ -159,7 +158,7 @@ async def test_capture_fetches_https_subresource_assets(
     assert any("asset" in entry for entry in replay["console"])
 
 
-@pytest.mark.uservice_oneshot(config_hooks=[_enable_https_only])
+@pytest.mark.uservice_oneshot(config_hooks=[enable_https_only])
 @pytest.mark.asyncio
 async def test_https_only_blocks_http_subresource_fetch(
     service_client, browser_probe, download_wacz, service_baseurl

webshotd/test/conftest.py

@@ -1,4 +1,5 @@
 import asyncio
+import copy
 import pathlib
 import shutil
 import tempfile
@@ -214,7 +215,7 @@ def userver_pg_config(pgsql_local, pg_gate):
     db_uri = {name: conn.get_uri() for name, conn in pgsql_local.items()}
     gate_host, gate_port = pg_gate.get_sockname_for_clients()
 
-    def _patch(config_yaml, _config_vars):
+    def _patch(config_yaml, config_vars):
         components = config_yaml["components_manager"]["components"]
         for component_name in ("capture_meta_db", "shared_state_db"):
             conninfo = db_uri[component_name]
@@ -226,6 +227,9 @@ def _patch(config_yaml, _config_vars):
             else:
                 netloc = f"{gate_host}:{gate_port}"
             updated = parsed._replace(netloc=netloc)
+            # Prefer config-vars for templated fields, but also patch the resolved YAML so
+            # the hook remains effective even if config-vars resolution happens earlier.
+            config_vars[f"pg_{component_name}_dsn"] = urlunparse(updated)
             component = components[component_name]
             component["dbconnection"] = urlunparse(updated)
             component.pop("dbalias", None)
@@ -261,7 +265,8 @@ def allowed_url_prefixes_extra(s3_gate_port):
 def patch_s3_config(s3_gate_port):
     del s3_gate_port
 
-    def _patch(config_yaml, _config_vars):
+    def _patch(config_yaml, config_vars):
+        del config_vars
         components = config_yaml["components_manager"]["components"]
         cfg = components["config"]
         cfg["s3_timeout_ms"] = _TESTSUITE_S3_TIMEOUT_MS
@@ -275,16 +280,22 @@ def _patch(config_yaml, _config_vars):
 
 @pytest.fixture(scope="session")
 def service_config_path_temp(
+    daemon_scoped_mark,
     service_tmpdir,
-    _service_config_hooked,
+    service_config_yaml,
+    service_config_vars,
     service_binary: pathlib.Path,
     service_source_dir: pathlib.Path,
     testsuite_webshotd_state_dir: pathlib.Path,
     s3_bucket_name: str,
 ) -> pathlib.Path:
+    # Mark this fixture as per-daemon. For oneshot tests, daemon_scoped_mark changes
+    # and pytest tears down/recreates the whole dependency tree.
+    del daemon_scoped_mark
+
     dst_path = service_tmpdir / "config.yaml"
-    config_yaml = dict(_service_config_hooked.config_yaml)
-    config_vars = dict(_service_config_hooked.config_vars)
+    config_yaml = copy.deepcopy(service_config_yaml)
+    config_vars = copy.deepcopy(service_config_vars)
     runtime_layout = runtime_layout_from_binary(service_binary)
     config_vars["rapidoc_assets_dir"] = str(runtime_layout.rapidoc_assets_dir)
     config_vars["openapi_public_dir"] = str(service_source_dir.parent / "schema" / "public")
@@ -295,6 +306,9 @@ def service_config_path_temp(
     config_vars["state_dir"] = str(testsuite_webshotd_state_dir)
     config_vars["s3_bucket"] = s3_bucket_name
     config_vars["public_base_url"] = f"http://127.0.0.1:8333/{s3_bucket_name}"
+    # Ensure url_bytes_max always exists in config_vars so oneshot config hooks may override it.
+    # static_config.yaml uses $url_bytes_max with a fallback; config vars must be concrete.
+    config_vars.setdefault("url_bytes_max", 32768)
     config_vars.update(task_processor_config_vars(config_yaml))
 
     components = config_yaml["components_manager"]["components"]

webshotd/test/deduplication.py

@@ -5,8 +5,9 @@
 from helper.waiters import wait_for_job_status
 
 
-def _patch_dedup_crawler_timeouts(_config_yaml, config_vars):
+def _patch_dedup_crawler_timeouts(config_yaml, config_vars):
     # Dedup performs two full crawl jobs in one test; give only this test a bit more crawl headroom.
+    del config_yaml
     config_vars["crawler_run_timeout_sec"] = 14
     config_vars["crawler_job_overhead_timeout_sec"] = 8
     config_vars["crawler_devtools_startup_timeout_sec"] = 10

webshotd/test/helper/capture_flow.py

@@ -6,16 +6,14 @@
 from zipfile import ZipFile
 
 import pytest
+from helper.config_hooks import enable_allowlist_only, enable_https_only
 from helper.waiters import wait_for_job_status
 from minio import Minio
 
+_enable_allowlist_only = enable_allowlist_only
 
-def _enable_allowlist_only(_config_yaml, config_vars):
-    config_vars["allowlist_only"] = True
 
-
-def _enable_https_only(_config_yaml, config_vars):
-    config_vars["https_only"] = True
+_enable_https_only = enable_https_only
 
 
 def _assert_missing_job_fields(job: dict, *names: str) -> None:

webshotd/test/helper/config_hooks.py

@@ -0,0 +1,43 @@
+def _require_components(config_yaml) -> dict:
+    try:
+        components = config_yaml["components_manager"]["components"]
+    except KeyError as exc:
+        raise RuntimeError("config_yaml missing components_manager.components") from exc
+    if not isinstance(components, dict):
+        raise RuntimeError("config_yaml components_manager.components must be a dict")
+    return components
+
+
+def set_component_field(config_yaml, component: str, field: str, value) -> None:
+    components = _require_components(config_yaml)
+    cfg = components.get(component)
+    if not isinstance(cfg, dict):
+        raise RuntimeError(f"component {component!r} config must be a dict")
+    cfg[field] = value
+
+
+def set_config_field(config_yaml, field: str, value) -> None:
+    set_component_field(config_yaml, "config", field, value)
+
+
+def set_config_var(config_vars, name: str, value) -> None:
+    config_vars[name] = value
+
+
+def set_config_var_and_field(config_yaml, config_vars, var_name: str, field: str, value) -> None:
+    # Prefer config_vars, but also patch config_yaml for robustness: some testsuite paths
+    # may write config_vars once per session, while per-test oneshot hooks still need to apply.
+    set_config_var(config_vars, var_name, value)
+    set_config_field(config_yaml, field, value)
+
+
+def enable_allowlist_only(config_yaml, config_vars) -> None:
+    set_config_var_and_field(config_yaml, config_vars, "allowlist_only", "allowlist_only", True)
+
+
+def enable_https_only(config_yaml, config_vars) -> None:
+    set_config_var_and_field(config_yaml, config_vars, "https_only", "https_only", True)
+
+
+def set_url_bytes_max(config_yaml, config_vars, value: int) -> None:
+    set_config_var_and_field(config_yaml, config_vars, "url_bytes_max", "url_bytes_max", value)