Merge pull request theupdateframework#259 from wittrock/develop

Enable IPv6 zone ID support for HttpRepository

Cargo.toml

@@ -29,6 +29,7 @@ itoa = "0.4"
 log = "0.4"
 ring = { version = "0.16" }
 parking_lot = "0.9"
+percent-encoding = "2.1"
 serde = "1"
 serde_derive = "1"
 serde_json = "1"

src/client.rs

@@ -29,8 +29,8 @@
 //!
 //! let local = FileSystemRepository::<Json>::new(PathBuf::from("~/.rustup"))?;
 //!
-//! let remote = HttpRepositoryBuilder::new(
-//!     url::Url::parse("https://static.rust-lang.org/").unwrap(),
+//! let remote = HttpRepositoryBuilder::new_with_uri(
+//!     "https://static.rust-lang.org/".parse::<http::Uri>().unwrap(),
 //!     HttpClient::new(),
 //! )
 //! .user_agent("rustup/1.4.0")

src/repository.rs

@@ -12,13 +12,15 @@ use hyper::Client;
 use hyper::Request;
 use log::debug;
 use parking_lot::RwLock;
+use percent_encoding::utf8_percent_encode;
 use std::collections::HashMap;
 use std::fs::{DirBuilder, File};
 use std::io;
 use std::marker::PhantomData;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use tempfile::{self, NamedTempFile};
+use url::Url;
 
 use crate::crypto::{self, HashAlgorithm, HashValue};
 use crate::error::Error;
@@ -28,7 +30,6 @@ use crate::metadata::{
 };
 use crate::util::SafeReader;
 use crate::Result;
-use url::Url;
 
 /// Top-level trait that represents a TUF repository and contains all the ways it can be interacted
 /// with.
@@ -372,7 +373,7 @@ where
     C: Connect + Sync + 'static,
     D: DataInterchange,
 {
-    url: Url,
+    uri: Uri,
     client: Client<C>,
     interchange: PhantomData<D>,
     user_agent: Option<String>,
@@ -389,7 +390,20 @@ where
     /// Create a new repository with the given `Url` and `Client`.
     pub fn new(url: Url, client: Client<C>) -> Self {
         HttpRepositoryBuilder {
-            url: url,
+            uri: url.to_string().parse::<Uri>().unwrap(), // This is dangerous, but will only exist for a short time as we migrate APIs.
+            client: client,
+            interchange: PhantomData,
+            user_agent: None,
+            metadata_prefix: None,
+            targets_prefix: None,
+            min_bytes_per_second: 4096,
+        }
+    }
+
+    /// Create a new repository with the given `Url` and `Client`.
+    pub fn new_with_uri(uri: Uri, client: Client<C>) -> Self {
+        HttpRepositoryBuilder {
+            uri: uri,
             client: client,
             interchange: PhantomData,
             user_agent: None,
@@ -443,7 +457,7 @@ where
         };
 
         HttpRepository {
-            url: self.url,
+            uri: self.uri,
             client: self.client,
             interchange: self.interchange,
             user_agent: user_agent,
@@ -460,7 +474,7 @@ where
     C: Connect + Sync + 'static,
     D: DataInterchange,
 {
-    url: Url,
+    uri: Uri,
     client: Client<C>,
     user_agent: String,
     metadata_prefix: Option<Vec<String>>,
@@ -469,6 +483,72 @@ where
     interchange: PhantomData<D>,
 }
 
+// Configuration for urlencoding URI path elements.
+// From https://url.spec.whatwg.org/#path-percent-encode-set
+const URLENCODE_FRAGMENT: &percent_encoding::AsciiSet = &percent_encoding::CONTROLS
+    .add(b' ')
+    .add(b'"')
+    .add(b'<')
+    .add(b'>')
+    .add(b'`');
+const URLENCODE_PATH: &percent_encoding::AsciiSet =
+    &URLENCODE_FRAGMENT.add(b'#').add(b'?').add(b'{').add(b'}');
+
+fn extend_uri(uri: Uri, prefix: &Option<Vec<String>>, components: &[String]) -> Result<Uri> {
+    let mut uri_parts = uri.into_parts();
+
+    let (path, query) = match &uri_parts.path_and_query {
+        Some(path_and_query) => (path_and_query.path(), path_and_query.query()),
+        None => ("", None),
+    };
+
+    let mut modified_path = path.to_owned();
+    if modified_path.ends_with("/") {
+        modified_path.pop();
+    }
+
+    let mut path_split = modified_path
+        .split("/")
+        .map(String::from)
+        .collect::<Vec<_>>();
+    let mut new_path_elements: Vec<&str> = vec![];
+
+    if let Some(ref prefix) = prefix {
+        new_path_elements.extend(prefix.iter().map(String::as_str));
+    }
+    new_path_elements.extend(components.iter().map(String::as_str));
+
+    // Urlencode new items to match behavior of PathSegmentsMut.extend from
+    // https://docs.rs/url/2.1.0/url/struct.PathSegmentsMut.html
+    let encoded_new_path_elements = new_path_elements
+        .into_iter()
+        .map(|path_segment| utf8_percent_encode(&path_segment, URLENCODE_PATH).collect());
+    path_split.extend(encoded_new_path_elements);
+    let constructed_path = path_split.join("/");
+
+    uri_parts.path_and_query =
+        match query {
+            Some(query) => Some(format!("{}?{}", constructed_path, query).parse().map_err(
+                |_| {
+                    Error::IllegalArgument(format!(
+                        "Invalid path and query: {:?}, {:?}",
+                        constructed_path, query
+                    ))
+                },
+            )?),
+            None => Some(constructed_path.parse().map_err(|_| {
+                Error::IllegalArgument(format!("Invalid URI path: {:?}", constructed_path))
+            })?),
+        };
+
+    Ok(Uri::from_parts(uri_parts).map_err(|_| {
+        Error::IllegalArgument(format!(
+            "Invalid URI parts: {:?}, {:?}, {:?}",
+            constructed_path, prefix, components
+        ))
+    })?)
+}
+
 impl<C, D> HttpRepository<C, D>
 where
     C: Connect + Sync + 'static,
@@ -479,20 +559,8 @@ where
         prefix: &'a Option<Vec<String>>,
         components: &'a [String],
     ) -> Result<Response<Body>> {
-        let mut url = self.url.clone();
-        {
-            let mut segments = url.path_segments_mut().map_err(|_| {
-                Error::IllegalArgument(format!("URL was 'cannot-be-a-base': {:?}", self.url))
-            })?;
-            if let Some(ref prefix) = prefix {
-                segments.extend(prefix);
-            }
-            segments.extend(components);
-        }
-
-        let uri: Uri = url.into_string().parse().map_err(|_| {
-            Error::IllegalArgument(format!("URL was 'cannot-be-a-base': {:?}", self.url))
-        })?;
+        let base_uri = self.uri.clone();
+        let uri = extend_uri(base_uri, prefix, components)?;
 
         let req = Request::builder()
             .uri(uri)
@@ -508,7 +576,7 @@ where
             } else {
                 Err(Error::Opaque(format!(
                     "Error getting {:?}: {:?}",
-                    self.url, resp
+                    self.uri, resp
                 )))
             }
         } else {
@@ -774,6 +842,148 @@ mod test {
     use futures_executor::block_on;
     use tempfile;
 
+    // Old behavior of the `HttpRepository::get` extension
+    // functionality
+    fn http_repository_extend_using_url(
+        base_url: Url,
+        prefix: &Option<Vec<String>>,
+        components: &[String],
+    ) -> url::Url {
+        let mut url = base_url.clone();
+        {
+            let mut segments = url.path_segments_mut().unwrap();
+            if let Some(ref prefix) = prefix {
+                segments.extend(prefix);
+            }
+            segments.extend(components);
+        }
+        return url;
+    }
+
+    #[test]
+    fn http_repository_uri_construction() {
+        let base_uri = "http://example.com/one";
+
+        let prefix = Some(vec![String::from("prefix")]);
+        let components = [
+            String::from("components_one"),
+            String::from("components_two"),
+        ];
+
+        let uri = base_uri.parse::<Uri>().unwrap();
+        let extended_uri = extend_uri(uri, &prefix, &components).unwrap();
+
+        let url =
+            http_repository_extend_using_url(Url::parse(base_uri).unwrap(), &prefix, &components);
+
+        assert_eq!(url.to_string(), extended_uri.to_string());
+        assert_eq!(
+            extended_uri.to_string(),
+            "http://example.com/one/prefix/components_one/components_two"
+        );
+    }
+
+    #[test]
+    fn http_repository_uri_construction_encoded() {
+        let base_uri = "http://example.com/one";
+
+        let prefix = Some(vec![String::from("prefix")]);
+        let components = [String::from("chars to encode#?")];
+        let uri = base_uri.parse::<Uri>().unwrap();
+        let extended_uri = extend_uri(uri, &prefix, &components)
+            .expect("correctly generated a URI with a zone id");
+
+        let url =
+            http_repository_extend_using_url(Url::parse(base_uri).unwrap(), &prefix, &components);
+
+        assert_eq!(url.to_string(), extended_uri.to_string());
+        assert_eq!(
+            extended_uri.to_string(),
+            "http://example.com/one/prefix/chars%20to%20encode%23%3F"
+        );
+    }
+
+    #[test]
+    fn http_repository_uri_construction_no_components() {
+        let base_uri = "http://example.com/one";
+
+        let prefix = Some(vec![String::from("prefix")]);
+        let components = [];
+
+        let uri = base_uri.parse::<Uri>().unwrap();
+        let extended_uri = extend_uri(uri, &prefix, &components).unwrap();
+
+        let url =
+            http_repository_extend_using_url(Url::parse(base_uri).unwrap(), &prefix, &components);
+
+        assert_eq!(url.to_string(), extended_uri.to_string());
+        assert_eq!(extended_uri.to_string(), "http://example.com/one/prefix");
+    }
+
+    #[test]
+    fn http_repository_uri_construction_no_prefix() {
+        let base_uri = "http://example.com/one";
+
+        let prefix = None;
+        let components = [
+            String::from("components_one"),
+            String::from("components_two"),
+        ];
+
+        let uri = base_uri.parse::<Uri>().unwrap();
+        let extended_uri = extend_uri(uri, &prefix, &components).unwrap();
+
+        let url =
+            http_repository_extend_using_url(Url::parse(base_uri).unwrap(), &prefix, &components);
+
+        assert_eq!(url.to_string(), extended_uri.to_string());
+        assert_eq!(
+            extended_uri.to_string(),
+            "http://example.com/one/components_one/components_two"
+        );
+    }
+
+    #[test]
+    fn http_repository_uri_construction_with_query() {
+        let base_uri = "http://example.com/one?test=1";
+
+        let prefix = None;
+        let components = [
+            String::from("components_one"),
+            String::from("components_two"),
+        ];
+
+        let uri = base_uri.parse::<Uri>().unwrap();
+        let extended_uri = extend_uri(uri, &prefix, &components).unwrap();
+
+        let url =
+            http_repository_extend_using_url(Url::parse(base_uri).unwrap(), &prefix, &components);
+
+        assert_eq!(url.to_string(), extended_uri.to_string());
+        assert_eq!(
+            extended_uri.to_string(),
+            "http://example.com/one/components_one/components_two?test=1"
+        );
+    }
+
+    #[test]
+    fn http_repository_uri_construction_ipv6_zoneid() {
+        let base_uri = "http://[aaaa::aaaa:aaaa:aaaa:1234%252]:80";
+
+        let prefix = Some(vec![String::from("prefix")]);
+        let components = [
+            String::from("componenents_one"),
+            String::from("components_two"),
+        ];
+        let uri = base_uri.parse::<Uri>().unwrap();
+        let extended_uri = extend_uri(uri, &prefix, &components)
+            .expect("correctly generated a URI with a zone id");
+        assert_eq!(
+            extended_uri.to_string(),
+            "http://[aaaa::aaaa:aaaa:aaaa:1234%252]:80/prefix/componenents_one/components_two"
+        );
+    }
+
     #[test]
     fn ephemeral_repo_targets() {
         block_on(async {