Skip to content

Migrate utility layer to HTTP types (PR 11)#623

Open
prk-Jr wants to merge 3 commits intofeature/edgezero-pr10-abstract-logging-initializationfrom
feature/edgezero-pr11-utility-layer-migration-v2
Open

Migrate utility layer to HTTP types (PR 11)#623
prk-Jr wants to merge 3 commits intofeature/edgezero-pr10-abstract-logging-initializationfrom
feature/edgezero-pr11-utility-layer-migration-v2

Conversation

@prk-Jr
Copy link
Copy Markdown
Collaborator

@prk-Jr prk-Jr commented Apr 8, 2026

Summary

  • Migrate the PR11 utility layer off direct fastly::Request/fastly::Response usage so core helpers can operate on http::{Request, Response} and edgezero_core::Body.
  • Add a temporary compat bridge at Fastly boundaries so handlers and integrations can keep working while later migration PRs move the remaining call stack.
  • Lock in the migration with focused compat tests and a guard test that prevents the migrated utility modules from drifting back to Fastly request/response types.

Changes

File Change
Cargo.toml Add the workspace mime dependency used by migrated HTTP response helpers.
Cargo.lock Record the new mime dependency.
crates/trusted-server-adapter-fastly/src/main.rs Route forwarded-header sanitization and basic-auth response conversion through the new compat boundary.
crates/trusted-server-core/Cargo.toml Add the core crate's mime workspace dependency.
crates/trusted-server-core/src/auction/endpoints.rs Convert auction utility calls to use the HTTP request compat bridge for synthetic ID and consent handling.
crates/trusted-server-core/src/auction/formats.rs Bridge Fastly requests into migrated synthetic ID generation helpers.
crates/trusted-server-core/src/auth.rs Migrate basic-auth enforcement to http::Request/Response and update tests to HTTP builders.
crates/trusted-server-core/src/compat.rs Add Fastly↔HTTP request/response conversions plus temporary Fastly boundary shims for headers, cookies, and synthetic cookie handling.
crates/trusted-server-core/src/consent/extraction.rs Migrate consent signal extraction to http::Request<EdgeBody>.
crates/trusted-server-core/src/consent/mod.rs Move consent pipeline input types and tests onto HTTP request types.
crates/trusted-server-core/src/cookies.rs Migrate cookie parsing/forwarding and synthetic cookie response helpers to HTTP request/response types.
crates/trusted-server-core/src/http_util.rs Migrate request/response helpers to HTTP types, preserve duplicate headers, and keep request-info logic on ClientInfo.
crates/trusted-server-core/src/integrations/lockr.rs Use Fastly compat shims for header/cookie forwarding at the integration boundary.
crates/trusted-server-core/src/integrations/permutive.rs Use Fastly compat shims for custom-header forwarding at the integration boundary.
crates/trusted-server-core/src/integrations/prebid.rs Bridge request-info construction and cookie forwarding through compat conversions.
crates/trusted-server-core/src/integrations/registry.rs Use compat conversions for synthetic ID generation and synthetic cookie response handling.
crates/trusted-server-core/src/integrations/testlight.rs Bridge the migrated synthetic ID lookup into the Fastly integration path.
crates/trusted-server-core/src/lib.rs Export the compat module and add migration guard tests.
crates/trusted-server-core/src/migration_guards.rs Add a regression test preventing migrated utility modules from reintroducing direct Fastly request/response types.
crates/trusted-server-core/src/proxy.rs Bridge proxy synthetic ID reads through the migrated HTTP utility layer.
crates/trusted-server-core/src/publisher.rs Route TSJS serving, request-info extraction, consent handling, and synthetic cookie writes through compat conversions.
crates/trusted-server-core/src/request_signing/endpoints.rs Switch JSON content-type constants to mime::APPLICATION_JSON.
crates/trusted-server-core/src/synthetic.rs Migrate synthetic ID helpers and tests to http::Request<EdgeBody>.

Closes

Closes #492

Test plan

  • cargo test --workspace
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo fmt --all -- --check
  • JS tests: cd crates/js/lib && npx vitest run
  • JS format: cd crates/js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve
  • Other: focused Rust verification with cargo test --package trusted-server-core compat -- --nocapture, cargo test --package trusted-server-core http_util -- --nocapture, cargo test --package trusted-server-core request_signing -- --nocapture, and cargo test --package trusted-server-core migration_guards -- --nocapture
  • Other: local cd crates/js/lib && npx vitest run currently fails before test execution with ERR_REQUIRE_ESM in html-encoding-sniffer -> @exodus/bytes/encoding-lite.js; leaving CI to capture the current JS environment issue.

Hardening note

This PR does not add any new config-derived regex or pattern compilation paths. Basic auth still surfaces invalid enabled handler regex configuration as an error rather than panicking, covered by auth::tests::returns_error_for_invalid_handler_regex_without_panicking alongside the existing settings startup validation tests.

Checklist

  • Changes follow CLAUDE.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses project logging macros (not println!)
  • New code has tests
  • No secrets or credentials committed

@prk-Jr prk-Jr self-assigned this Apr 8, 2026
@prk-Jr prk-Jr linked an issue Apr 8, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Collaborator

@ChristianPavilonis ChristianPavilonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This PR cleanly migrates utility-layer functions (auth, cookies, synthetic, http_util, consent/extraction, consent/mod) from fastly::Request/fastly::Response to http::Request/http::Response with EdgeBody. The compat bridge pattern is sound and well-tested.

Two duplicate-header-dropping bugs need fixing before merge — see inline comments.

Blocking

🔧 wrench

  • to_fastly_request drops duplicate headers: Uses set_header instead of append_header, losing multi-valued headers during conversion (compat.rs:65)
  • copy_custom_headers drops duplicate X-headers: Uses insert instead of append, a behavioral regression from the old Fastly-based version (http_util.rs:22)

Non-blocking

🤔 thinking

  • Migration guard strip_line_comments is naive about string literals: A Rust string literal like "fastly::Request" in a test assertion would trigger a false positive. Currently does not happen, but the approach is fragile. Consider adding a brief comment documenting the limitation. (migration_guards.rs)

🌱 seedling

  • copy_custom_headers in http_util.rs has no callers outside tests: The integrations (lockr, permutive) now call compat::copy_fastly_custom_headers instead. Worth noting for PR 15 cleanup.

⛏ nitpick

  • Dead "X-" check in copy_fastly_custom_headers: The check for name_str.starts_with("X-") in compat.rs:144 is dead code since Fastly normalizes header names to lowercase. The migrated copy_custom_headers in http_util.rs only checks "x-". Not a correctness issue, but the asymmetry may confuse readers.
  • Temporary compat functions well-documented with removal targets: Every public function in compat.rs has a # PR 15 removal target annotation — great practice for tracking temporary code.

👍 praise

  • Thorough compat test coverage: 11 focused tests covering round-trip conversions, duplicate header preservation, header sanitization, cookie forwarding with consent stripping, and synthetic cookie lifecycle. Excellent scaffolding for temporary bridge code.
  • Migration guard test is a clever regression barrier: The include_str! approach to scan source files for banned Fastly types provides a compile-time-like guarantee without proc macros.

📝 note

  • mime dependency is appropriate: Replaces fastly::mime::APPLICATION_JSON references. Well-maintained, zero-dependency crate suitable for the use case.

CI Status

  • All checks: PASS

Comment thread crates/trusted-server-core/src/compat.rs Outdated
Comment thread crates/trusted-server-core/src/http_util.rs Outdated
Copy link
Copy Markdown
Collaborator

@aram356 aram356 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Migrates the utility layer (auth, cookies, synthetic, http_util, consent) off direct fastly::Request/fastly::Response to http::{Request, Response}<EdgeBody> with a temporary compat bridge. Well-structured migration with good test coverage and clear lifecycle annotations. Two header-handling bugs need fixing before merge.

Blocking

🔧 wrench

  • copy_custom_headers drops duplicate headers: uses insert instead of append in http_util.rs:22. The compat shim preserves duplicates; this migrated version doesn't. Will become a production regression when the compat layer is removed in PR 15.
  • to_fastly_request drops duplicate headers: uses set_header instead of append_header in compat.rs:65. Inconsistent with to_fastly_response which correctly uses append_header on line 109.

Non-blocking

🤔 thinking

  • Migration guard doesn't handle block comments: strip_line_comments in migration_guards.rs only strips // line comments. A /* fastly::Request */ block comment would cause a false positive, and a real regression hidden inside a block comment wouldn't be caught. Acceptable for a guard test (false positives are safe), just noting the limitation.

🌱 seedling

  • Add a to_fastly_request duplicate-header round-trip test: after fixing the append_header issue, a round-trip test (http::Requestfastly::Requesthttp::Request) verifying duplicate header preservation would prevent future regressions.

⛏ nitpick

  • Redundant case check in copy_fastly_custom_headers: compat.rs:144 checks both "x-" and "X-" but Fastly's HeaderName is already case-normalized to lowercase. Not worth changing in temporary code.

CI Status

  • integration tests: PASS
  • browser integration tests: PASS
  • prepare integration artifacts: PASS

Comment thread crates/trusted-server-core/src/http_util.rs Outdated
Comment thread crates/trusted-server-core/src/compat.rs Outdated
Comment thread crates/trusted-server-core/src/compat.rs Outdated
Copy link
Copy Markdown
Collaborator

@aram356 aram356 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Prior blocking findings (duplicate-header drops in to_fastly_request and copy_custom_headers) are both fixed in 2817761, with round-trip regression tests added. CI is green. Approving; non-blocking observations below.

Non-blocking

🤔 thinking

  • from_fastly_request_ref silently drops body (crates/trusted-server-core/src/compat.rs:54): docs note the body is empty, but the name doesn't signal it. Footgun if a future caller passes a POST expecting body access. Consider renaming to from_fastly_headers_ref or adding a debug_assert! when the source request has a non-empty body. All current callers only read headers, so not a bug today.
  • Double conversion on the auction hot path: auction/endpoints.rs:50 builds http_req once, but auction/formats.rs:93 (convert_tsjs_to_auction_request) re-converts the same underlying fastly::Request again via from_fastly_request_ref. Each conversion iterates all headers — two copies per auction request. Acceptable for a temporary bridge (PR 15 removes it), but worth a comment referencing PR 15 so the duplication doesn't get frozen.
  • forward_cookie_header uses insert, not append (crates/trusted-server-core/src/cookies.rs:167,175,184): fine for HTTP/1 where Cookie is a single semicolon-joined value, but HTTP/2 (RFC 7540 §8.1.2.5) allows splitting across multiple headers and only the first would be forwarded. Matches pre-migration behavior and Fastly concatenates HTTP/2 cookie headers upstream, so likely not a live bug — worth documenting the assumption.

🌱 seedling

  • Migration guard scope is "utility modules only" (crates/trusted-server-core/src/migration_guards.rs:27-37): guards 6 files. Handler/integration files (publisher.rs, proxy.rs, registry.rs, integrations/*.rs, auction/*.rs) still use fastly::Request by design. PR 12–14 should extend the banned-patterns list as files are migrated.

⛏ nitpick

  • Dead case check in copy_fastly_custom_headers (crates/trusted-server-core/src/compat.rs:144): checks both "x-" and "X-" though fastly::Request normalizes to lowercase. Not worth a commit in temporary code — trim on the next touch.

CI Status

  • integration tests: PASS
  • browser integration tests: PASS
  • prepare integration artifacts: PASS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Utility layer type migration + compat adapter

3 participants