Merge pull request #2241 from input-output-hk/ensemble/aggregator/ref…

…actor_url_sanitizer

refactor(aggregator): better url sanitization process by using a value object

mithril-aggregator/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-aggregator"
-version = "0.6.18"
+version = "0.6.19"
 description = "A Mithril Aggregator server"
 authors = { workspace = true }
 edition = { workspace = true }

mithril-aggregator/src/artifact_builder/cardano_database.rs

@@ -133,12 +133,12 @@ mod tests {
         test_utils::{fake_data, TempDir},
         CardanoNetwork,
     };
-    use reqwest::Url;
 
     use crate::{
         artifact_builder::{MockAncillaryFileUploader, MockImmutableFilesUploader},
         immutable_file_digest_mapper::MockImmutableFileDigestMapper,
         test_tools::TestLogger,
+        tools::url_sanitizer::SanitizedUrlWithTrailingSlash,
         DumbSnapshotter,
     };
 
@@ -238,7 +238,7 @@ mod tests {
                 .returning(|| Ok(BTreeMap::new()));
 
             DigestArtifactBuilder::new(
-                Url::parse("http://aggregator_uri").unwrap(),
+                SanitizedUrlWithTrailingSlash::parse("http://aggregator_uri").unwrap(),
                 vec![],
                 test_dir.join("digests"),
                 Arc::new(immutable_file_digest_mapper),

mithril-aggregator/src/artifact_builder/cardano_database_artifacts/digest.rs

@@ -10,11 +10,11 @@ use mithril_common::{
     entities::DigestLocation, logging::LoggerExtensions,
     messages::CardanoDatabaseDigestListItemMessage, StdResult,
 };
-use reqwest::Url;
 use slog::{error, Logger};
 
 use crate::{
     file_uploaders::{GcpUploader, LocalUploader},
+    tools::url_sanitizer::SanitizedUrlWithTrailingSlash,
     DumbUploader, FileUploader, ImmutableFileDigestMapper,
 };
 
@@ -55,7 +55,8 @@ impl DigestFileUploader for GcpUploader {
 
 pub struct DigestArtifactBuilder {
     /// Aggregator URL prefix
-    aggregator_url_prefix: Url,
+    aggregator_url_prefix: SanitizedUrlWithTrailingSlash,
+
     /// Uploaders
     uploaders: Vec<Arc<dyn DigestFileUploader>>,
 
@@ -69,7 +70,7 @@ pub struct DigestArtifactBuilder {
 impl DigestArtifactBuilder {
     /// Creates a new [DigestArtifactBuilder].
     pub fn new(
-        aggregator_url_prefix: Url,
+        aggregator_url_prefix: SanitizedUrlWithTrailingSlash,
         uploaders: Vec<Arc<dyn DigestFileUploader>>,
         digests_dir: PathBuf,
         immutable_file_digest_mapper: Arc<dyn ImmutableFileDigestMapper>,
@@ -211,7 +212,7 @@ mod tests {
             .returning(|| Ok(BTreeMap::new()));
 
         let builder = DigestArtifactBuilder::new(
-            Url::parse("https://aggregator/").unwrap(),
+            SanitizedUrlWithTrailingSlash::parse("https://aggregator/").unwrap(),
             vec![],
             temp_dir,
             Arc::new(immutable_file_digest_mapper),
@@ -240,7 +241,7 @@ mod tests {
 
         {
             let builder = DigestArtifactBuilder::new(
-                Url::parse("https://aggregator/").unwrap(),
+                SanitizedUrlWithTrailingSlash::parse("https://aggregator/").unwrap(),
                 vec![Arc::new(uploader)],
                 PathBuf::from("/tmp/whatever"),
                 Arc::new(MockImmutableFileDigestMapper::new()),
@@ -260,7 +261,7 @@ mod tests {
         let uploader = fake_uploader_returning_error();
 
         let builder = DigestArtifactBuilder::new(
-            Url::parse("https://aggregator/").unwrap(),
+            SanitizedUrlWithTrailingSlash::parse("https://aggregator/").unwrap(),
             vec![Arc::new(uploader)],
             PathBuf::from("/tmp/whatever"),
             Arc::new(MockImmutableFileDigestMapper::new()),
@@ -289,7 +290,7 @@ mod tests {
         ];
 
         let builder = DigestArtifactBuilder::new(
-            Url::parse("https://aggregator/").unwrap(),
+            SanitizedUrlWithTrailingSlash::parse("https://aggregator/").unwrap(),
             uploaders,
             PathBuf::from("/tmp/whatever"),
             Arc::new(MockImmutableFileDigestMapper::new()),
@@ -324,7 +325,7 @@ mod tests {
             vec![Arc::new(first_uploader), Arc::new(second_uploader)];
 
         let builder = DigestArtifactBuilder::new(
-            Url::parse("https://aggregator/").unwrap(),
+            SanitizedUrlWithTrailingSlash::parse("https://aggregator/").unwrap(),
             uploaders,
             PathBuf::from("/tmp/whatever"),
             Arc::new(MockImmutableFileDigestMapper::new()),
@@ -370,7 +371,7 @@ mod tests {
             });
 
         let builder = DigestArtifactBuilder::new(
-            Url::parse("https://aggregator/").unwrap(),
+            SanitizedUrlWithTrailingSlash::parse("https://aggregator/").unwrap(),
             vec![],
             temp_dir,
             Arc::new(immutable_file_digest_mapper),
@@ -421,7 +422,7 @@ mod tests {
             });
 
         let builder = DigestArtifactBuilder::new(
-            Url::parse("https://aggregator/").unwrap(),
+            SanitizedUrlWithTrailingSlash::parse("https://aggregator/").unwrap(),
             vec![Arc::new(digest_file_uploader)],
             digests_dir,
             Arc::new(immutable_file_digest_mapper),

mithril-aggregator/src/artifact_builder/cardano_database_artifacts/immutable.rs

@@ -734,8 +734,8 @@ mod tests {
         use std::fs::File;
         use std::io::Write;
 
+        use crate::tools::url_sanitizer::SanitizedUrlWithTrailingSlash;
         use mithril_common::test_utils::TempDir;
-        use reqwest::Url;
 
         use super::*;
 
@@ -765,7 +765,8 @@ mod tests {
             let archive_1 = create_fake_archive(&source_dir, "00001.tar.gz");
             let archive_2 = create_fake_archive(&source_dir, "00002.tar.gz");
 
-            let url_prefix = Url::parse("http://test.com:8080/base-root").unwrap();
+            let url_prefix =
+                SanitizedUrlWithTrailingSlash::parse("http://test.com:8080/base-root").unwrap();
             let uploader =
                 LocalUploader::new(url_prefix, &target_dir, TestLogger::stdout()).unwrap();
             let location = ImmutableFilesUploader::batch_upload(
@@ -799,7 +800,8 @@ mod tests {
 
             let archive = create_fake_archive(&source_dir, "not-templatable.tar.gz");
 
-            let url_prefix = Url::parse("http://test.com:8080/base-root").unwrap();
+            let url_prefix =
+                SanitizedUrlWithTrailingSlash::parse("http://test.com:8080/base-root").unwrap();
             let uploader =
                 LocalUploader::new(url_prefix, &target_dir, TestLogger::stdout()).unwrap();
 

mithril-aggregator/src/configuration.rs

@@ -4,7 +4,6 @@ use mithril_common::chain_observer::ChainObserverType;
 use mithril_common::crypto_helper::ProtocolGenesisSigner;
 use mithril_common::era::adapters::EraReaderAdapterType;
 use mithril_doc::{Documenter, DocumenterDefault, StructDoc};
-use reqwest::Url;
 use serde::{Deserialize, Serialize};
 use std::collections::{BTreeSet, HashMap};
 use std::path::PathBuf;
@@ -18,7 +17,7 @@ use mithril_common::entities::{
 use mithril_common::{CardanoNetwork, StdResult};
 
 use crate::http_server::SERVER_BASE_PATH;
-use crate::tools::url_sanitizer;
+use crate::tools::url_sanitizer::SanitizedUrlWithTrailingSlash;
 
 /// Different kinds of execution environments
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
@@ -282,20 +281,19 @@ impl Configuration {
     }
 
     /// Build the local server URL from configuration.
-    pub fn get_local_server_url(&self) -> StdResult<Url> {
-        let url = Url::parse(&format!(
+    pub fn get_local_server_url(&self) -> StdResult<SanitizedUrlWithTrailingSlash> {
+        SanitizedUrlWithTrailingSlash::parse(&format!(
             "http://{}:{}/{SERVER_BASE_PATH}/",
             self.server_ip, self.server_port
-        ))?;
-        Ok(url)
+        ))
     }
 
     /// Get the server URL from the configuration.
     ///
     /// Will return the public server URL if it is set, otherwise the local server URL.
-    pub fn get_server_url(&self) -> StdResult<Url> {
+    pub fn get_server_url(&self) -> StdResult<SanitizedUrlWithTrailingSlash> {
         match &self.public_server_url {
-            Some(url) => Ok(url_sanitizer::sanitize_url_path(&Url::parse(url)?)?),
+            Some(url) => SanitizedUrlWithTrailingSlash::parse(url),
             None => self.get_local_server_url(),
         }
     }

mithril-aggregator/src/dependency_injection/builder.rs

@@ -1265,10 +1265,7 @@ impl DependenciesBuilder {
                 SnapshotUploaderType::Local => {
                     let server_url_prefix = self.configuration.get_server_url()?;
                     let ancillary_url_prefix = server_url_prefix
-                        .join(&format!("{CARDANO_DATABASE_DOWNLOAD_PATH}/ancillary/"))
-                        .with_context(|| {
-                            format!("Could not join `{CARDANO_DATABASE_DOWNLOAD_PATH}/ancillary/` to URL `{server_url_prefix}`")
-                        })?;
+                        .sanitize_join(&format!("{CARDANO_DATABASE_DOWNLOAD_PATH}/ancillary/"))?;
                     let target_dir = self
                         .configuration
                         .get_snapshot_dir()?
@@ -1313,8 +1310,7 @@ impl DependenciesBuilder {
                 SnapshotUploaderType::Local => {
                     let server_url_prefix = self.configuration.get_server_url()?;
                     let immutable_url_prefix = server_url_prefix
-                        .join(&format!("{CARDANO_DATABASE_DOWNLOAD_PATH}/immutable/"))
-                        .with_context(|| format!("Could not join `{CARDANO_DATABASE_DOWNLOAD_PATH}/immutable/` to URL `{server_url_prefix}`"))?;
+                        .sanitize_join(&format!("{CARDANO_DATABASE_DOWNLOAD_PATH}/immutable/"))?;
                     let target_dir = self
                         .configuration
                         .get_snapshot_dir()?
@@ -1350,8 +1346,7 @@ impl DependenciesBuilder {
                 SnapshotUploaderType::Local => {
                     let server_url_prefix = self.configuration.get_server_url()?;
                     let digests_url_prefix = server_url_prefix
-                        .join(&format!("{CARDANO_DATABASE_DOWNLOAD_PATH}/digests/"))
-                        .with_context(|| format!("Could not join `{CARDANO_DATABASE_DOWNLOAD_PATH}/digests/` to URL `{server_url_prefix}`"))?;
+                        .sanitize_join(&format!("{CARDANO_DATABASE_DOWNLOAD_PATH}/digests/"))?;
                     let target_dir = self
                         .configuration
                         .get_snapshot_dir()?
@@ -1759,7 +1754,7 @@ impl DependenciesBuilder {
             dependency_container.clone(),
             RouterConfig {
                 network: self.configuration.get_network()?,
-                server_url: self.configuration.get_server_url()?.to_string(),
+                server_url: self.configuration.get_server_url()?,
                 allowed_discriminants: self.get_allowed_signed_entity_types_discriminants()?,
                 cardano_transactions_prover_max_hashes_allowed_by_request: self
                     .configuration

mithril-aggregator/src/file_uploaders/local_snapshot_uploader.rs

@@ -1,21 +1,20 @@
 use anyhow::Context;
 use async_trait::async_trait;
 use mithril_common::entities::FileUri;
-use reqwest::Url;
 use slog::{debug, Logger};
 use std::path::{Path, PathBuf};
 
 use mithril_common::logging::LoggerExtensions;
 use mithril_common::StdResult;
 
 use crate::file_uploaders::FileUploader;
-use crate::tools::{self, url_sanitizer::sanitize_url_path};
+use crate::tools::{self, url_sanitizer::SanitizedUrlWithTrailingSlash};
 
 // It's only used by the legacy snapshot that uploads the entire Cardano database.
 /// LocalSnapshotUploader is a file uploader working using local files
 pub struct LocalSnapshotUploader {
     /// File server URL prefix
-    server_url_prefix: Url,
+    server_url_prefix: SanitizedUrlWithTrailingSlash,
 
     /// Target folder where to store files archive
     target_location: PathBuf,
@@ -26,13 +25,12 @@ pub struct LocalSnapshotUploader {
 impl LocalSnapshotUploader {
     /// LocalSnapshotUploader factory
     pub(crate) fn new(
-        server_url_prefix: Url,
+        server_url_prefix: SanitizedUrlWithTrailingSlash,
         target_location: &Path,
         logger: Logger,
     ) -> StdResult<Self> {
         let logger = logger.new_with_component_name::<Self>();
         debug!(logger, "New LocalSnapshotUploader created"; "server_url_prefix" => &server_url_prefix.as_str());
-        let server_url_prefix = sanitize_url_path(&server_url_prefix)?;
 
         Ok(Self {
             server_url_prefix,
@@ -99,7 +97,8 @@ mod tests {
             &digest
         );
 
-        let url_prefix = Url::parse("http://test.com:8080/base-root").unwrap();
+        let url_prefix =
+            SanitizedUrlWithTrailingSlash::parse("http://test.com:8080/base-root").unwrap();
         let uploader =
             LocalSnapshotUploader::new(url_prefix, target_dir.path(), TestLogger::stdout())
                 .unwrap();
@@ -118,7 +117,7 @@ mod tests {
         let digest = "41e27b9ed5a32531b95b2b7ff3c0757591a06a337efaf19a524a998e348028e7";
         let archive = create_fake_archive(source_dir.path(), digest);
         let uploader = LocalSnapshotUploader::new(
-            Url::parse("http://test.com:8080/base-root/").unwrap(),
+            SanitizedUrlWithTrailingSlash::parse("http://test.com:8080/base-root/").unwrap(),
             target_dir.path(),
             TestLogger::stdout(),
         )
@@ -138,7 +137,7 @@ mod tests {
         create_fake_archive(source_dir.path(), digest);
         let target_dir = tempdir().unwrap();
         let uploader = LocalSnapshotUploader::new(
-            Url::parse("http://test.com:8080/base-root/").unwrap(),
+            SanitizedUrlWithTrailingSlash::parse("http://test.com:8080/base-root/").unwrap(),
             target_dir.path(),
             TestLogger::stdout(),
         )

mithril-aggregator/src/file_uploaders/local_uploader.rs

@@ -1,19 +1,18 @@
 use anyhow::Context;
 use async_trait::async_trait;
-use reqwest::Url;
 use slog::{debug, Logger};
 use std::path::{Path, PathBuf};
 
 use mithril_common::StdResult;
 use mithril_common::{entities::FileUri, logging::LoggerExtensions};
 
 use crate::file_uploaders::FileUploader;
-use crate::tools::url_sanitizer::sanitize_url_path;
+use crate::tools::url_sanitizer::SanitizedUrlWithTrailingSlash;
 
 /// LocalUploader is a file uploader working using local files
 pub struct LocalUploader {
     /// File server URL prefix
-    server_url_prefix: Url,
+    server_url_prefix: SanitizedUrlWithTrailingSlash,
 
     /// Target folder where to store files archive
     target_location: PathBuf,
@@ -24,13 +23,12 @@ pub struct LocalUploader {
 impl LocalUploader {
     /// LocalUploader factory
     pub(crate) fn new(
-        server_url_prefix: Url,
+        server_url_prefix: SanitizedUrlWithTrailingSlash,
         target_location: &Path,
         logger: Logger,
     ) -> StdResult<Self> {
         let logger = logger.new_with_component_name::<Self>();
         debug!(logger, "New LocalUploader created"; "server_url_prefix" => &server_url_prefix.as_str());
-        let server_url_prefix = sanitize_url_path(&server_url_prefix)?;
 
         Ok(Self {
             server_url_prefix,
@@ -101,7 +99,8 @@ mod tests {
             &archive.file_name().unwrap().to_str().unwrap()
         );
 
-        let url_prefix = Url::parse("http://test.com:8080/base-root").unwrap();
+        let url_prefix =
+            SanitizedUrlWithTrailingSlash::parse("http://test.com:8080/base-root").unwrap();
         let uploader = LocalUploader::new(url_prefix, &target_dir, TestLogger::stdout()).unwrap();
         let location = FileUploader::upload(&uploader, &archive)
             .await
@@ -123,7 +122,7 @@ mod tests {
         println!("target_dir: {:?}", target_dir);
         let archive = create_fake_archive(&source_dir, "an_archive");
         let uploader = LocalUploader::new(
-            Url::parse("http://test.com:8080/base-root/").unwrap(),
+            SanitizedUrlWithTrailingSlash::parse("http://test.com:8080/base-root/").unwrap(),
             &target_dir,
             TestLogger::stdout(),
         )
@@ -144,7 +143,7 @@ mod tests {
             "should_error_if_path_is_a_directory_target",
         );
         let uploader = LocalUploader::new(
-            Url::parse("http://test.com:8080/base-root/").unwrap(),
+            SanitizedUrlWithTrailingSlash::parse("http://test.com:8080/base-root/").unwrap(),
             &target_dir,
             TestLogger::stdout(),
         )