From 279384df349230bb5471b75dad736e9cc0b6e9c9 Mon Sep 17 00:00:00 2001 From: init4samwise Date: Fri, 13 Feb 2026 07:14:09 +0000 Subject: [PATCH 1/3] fix: auto-sanitize TX_POOL_URL trailing slash Automatically ensures TX_POOL_URL has a trailing slash during config sanitization, fixing url.join() behavior for endpoint construction. - Add sanitize() method to BuilderConfig - Apply sanitization on config load in config_from_env() - Remove manual trailing slash requirement from README - Add 6 unit tests for URL sanitization Closes ENG-1531 --- README.md | 2 +- src/config.rs | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 4 +- 3 files changed, 122 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b9d69f25..f60c7fc5 100644 --- a/README.md +++ b/README.md @@ -93,7 +93,7 @@ Key | Required | Description `ROLLUP_RPC_URL` | Yes | RPC endpoint for the rollup chain `QUINCEY_URL` | Yes | Remote sequencer signing endpoint `SEQUENCER_KEY` | No | AWS Key ID _OR_ local private key for the Sequencer; set IFF using local Sequencer signing instead of remote (via `QUINCEY_URL`) Quincey signing -`TX_POOL_URL` | Yes | Transaction pool URL (must end with `/`) +`TX_POOL_URL` | Yes | Transaction pool URL `FLASHBOTS_ENDPOINT` | No | Flashbots API to submit blocks to `ROLLUP_BLOCK_GAS_LIMIT` | No | Override for rollup block gas limit `MAX_HOST_GAS_COEFFICIENT` | No | Optional maximum host gas coefficient, as a percentage, to use when building blocks diff --git a/src/config.rs b/src/config.rs index 1296ccc0..0e7c5c3c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -171,6 +171,21 @@ pub struct BuilderConfig { } impl BuilderConfig { + /// Sanitizes URL fields to ensure consistent behavior with `url.join()`. + /// + /// This ensures `tx_pool_url` has a trailing slash so that `url.join("path")` + /// appends to the URL path rather than replacing the last segment. + /// For example: + /// - `http://example.com/api/` + `join("transactions")` = `http://example.com/api/transactions` + /// - `http://example.com/api` + `join("transactions")` = `http://example.com/transactions` (wrong!) + pub fn sanitize(mut self) -> Self { + // Ensure tx_pool_url has a trailing slash for correct url.join() behavior + if !self.tx_pool_url.path().ends_with('/') { + self.tx_pool_url.set_path(&format!("{}/", self.tx_pool_url.path())); + } + self + } + /// Connect to the Builder signer. pub async fn connect_builder_signer(&self) -> eyre::Result { static ONCE: tokio::sync::OnceCell = tokio::sync::OnceCell::const_new(); @@ -286,3 +301,106 @@ impl BuilderConfig { as u64 } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Tests that URL sanitization correctly handles trailing slashes for url.join() behavior. + /// + /// The `url.join()` method behaves differently based on trailing slashes: + /// - With trailing slash: `http://example.com/api/`.join("path") = `http://example.com/api/path` + /// - Without trailing slash: `http://example.com/api`.join("path") = `http://example.com/path` + /// + /// This test verifies that our sanitization ensures consistent behavior. + mod tx_pool_url_sanitization { + use url::Url; + + #[test] + fn url_without_trailing_slash_gets_sanitized() { + let url: Url = "http://localhost:9000".parse().unwrap(); + assert!(!url.path().ends_with('/')); + + // Simulate sanitization + let mut sanitized = url.clone(); + if !sanitized.path().ends_with('/') { + sanitized.set_path(&format!("{}/", sanitized.path())); + } + + assert!(sanitized.path().ends_with('/')); + assert_eq!(sanitized.as_str(), "http://localhost:9000/"); + } + + #[test] + fn url_with_trailing_slash_unchanged() { + let url: Url = "http://localhost:9000/".parse().unwrap(); + assert!(url.path().ends_with('/')); + + // Simulate sanitization - should be unchanged + let mut sanitized = url.clone(); + if !sanitized.path().ends_with('/') { + sanitized.set_path(&format!("{}/", sanitized.path())); + } + + assert_eq!(sanitized.as_str(), "http://localhost:9000/"); + } + + #[test] + fn url_with_path_without_trailing_slash_gets_sanitized() { + let url: Url = "http://localhost:9000/api/v1".parse().unwrap(); + assert!(!url.path().ends_with('/')); + + // Simulate sanitization + let mut sanitized = url.clone(); + if !sanitized.path().ends_with('/') { + sanitized.set_path(&format!("{}/", sanitized.path())); + } + + assert!(sanitized.path().ends_with('/')); + assert_eq!(sanitized.as_str(), "http://localhost:9000/api/v1/"); + } + + #[test] + fn url_with_path_with_trailing_slash_unchanged() { + let url: Url = "http://localhost:9000/api/v1/".parse().unwrap(); + assert!(url.path().ends_with('/')); + + // Simulate sanitization - should be unchanged + let mut sanitized = url.clone(); + if !sanitized.path().ends_with('/') { + sanitized.set_path(&format!("{}/", sanitized.path())); + } + + assert_eq!(sanitized.as_str(), "http://localhost:9000/api/v1/"); + } + + #[test] + fn sanitized_url_joins_correctly() { + // Without sanitization - WRONG behavior + let url_no_slash: Url = "http://localhost:9000/api".parse().unwrap(); + let joined_wrong = url_no_slash.join("transactions").unwrap(); + assert_eq!(joined_wrong.as_str(), "http://localhost:9000/transactions"); + + // With sanitization - CORRECT behavior + let url_with_slash: Url = "http://localhost:9000/api/".parse().unwrap(); + let joined_correct = url_with_slash.join("transactions").unwrap(); + assert_eq!(joined_correct.as_str(), "http://localhost:9000/api/transactions"); + } + + #[test] + fn sanitized_root_url_joins_correctly() { + // Root URL without trailing slash + let url_no_slash: Url = "http://localhost:9000".parse().unwrap(); + + // Sanitize it + let mut sanitized = url_no_slash.clone(); + if !sanitized.path().ends_with('/') { + sanitized.set_path(&format!("{}/", sanitized.path())); + } + + // Now join should work correctly + let joined = sanitized.join("transactions").unwrap(); + assert_eq!(joined.as_str(), "http://localhost:9000/transactions"); + } + } +} diff --git a/src/lib.rs b/src/lib.rs index b727a412..7c5ae2bf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,7 +50,9 @@ pub static CONFIG: OnceLock = OnceLock::new(); /// Panics if the configuration cannot be loaded from the environment AND no /// other configuration has been previously initialized. pub fn config_from_env() -> &'static config::BuilderConfig { - CONFIG.get_or_init(|| config::BuilderConfig::from_env().expect("Failed to load Builder config")) + CONFIG.get_or_init(|| { + config::BuilderConfig::from_env().expect("Failed to load Builder config").sanitize() + }) } /// Get a reference to the global Builder configuration. From 8a60004bc672308b13b4425249b7189aa6325a1e Mon Sep 17 00:00:00 2001 From: init4samwise Date: Fri, 13 Feb 2026 17:49:11 +0000 Subject: [PATCH 2/3] fix(config): correct test for URL trailing slash behavior The test url_without_trailing_slash_gets_sanitized was incorrectly asserting that parsing "http://localhost:9000" would give a URL without a trailing slash. Per URL spec, a URL without an explicit path gets the root path "/", which DOES end with a slash. Renamed test to root_url_already_has_trailing_slash and updated assertions to document this behavior correctly - the sanitization is actually a no-op for root URLs since they already have the trailing slash. --- src/config.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/config.rs b/src/config.rs index 0e7c5c3c..757a85a2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -317,11 +317,14 @@ mod tests { use url::Url; #[test] - fn url_without_trailing_slash_gets_sanitized() { + fn root_url_already_has_trailing_slash() { + // Per URL spec, a URL without an explicit path gets the root path "/" + // So "http://localhost:9000" is equivalent to "http://localhost:9000/" let url: Url = "http://localhost:9000".parse().unwrap(); - assert!(!url.path().ends_with('/')); + assert_eq!(url.path(), "/"); + assert!(url.path().ends_with('/')); - // Simulate sanitization + // Sanitization is a no-op for root URLs since they already have trailing slash let mut sanitized = url.clone(); if !sanitized.path().ends_with('/') { sanitized.set_path(&format!("{}/", sanitized.path())); From c7af846949753e39299e6df95d1c811567d8e5cc Mon Sep 17 00:00:00 2001 From: init4samwise Date: Fri, 13 Feb 2026 17:53:03 +0000 Subject: [PATCH 3/3] fix(config): remove unused import in tests --- src/config.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/config.rs b/src/config.rs index 757a85a2..62945af9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -304,8 +304,6 @@ impl BuilderConfig { #[cfg(test)] mod tests { - use super::*; - /// Tests that URL sanitization correctly handles trailing slashes for url.join() behavior. /// /// The `url.join()` method behaves differently based on trailing slashes: