Better Opaque Exact Match Error Handling and Docs (#64)

* Add notes about Opaque Origins

* Improve error handling for Opaque Exact matches

* Cleanups

* Add more test condition

src/headers.rs

@@ -217,6 +217,39 @@ mod tests {
         Client::new(rocket).expect("valid rocket instance")
     }
 
+    // `Origin::from_str` tests
+
+    #[test]
+    fn origin_is_parsed_properly() {
+        let url = "https://foo.bar.xyz";
+        let parsed = not_err!(Origin::from_str(url));
+        assert_eq!(parsed.ascii_serialization(), url);
+    }
+
+    #[test]
+    fn origin_parsing_strips_paths() {
+        // this should never really be sent by a compliant user agent
+        let url = "https://foo.bar.xyz/path/somewhere";
+        let parsed = not_err!(Origin::from_str(url));
+        let expected = "https://foo.bar.xyz";
+        assert_eq!(parsed.ascii_serialization(), expected);
+    }
+
+    #[test]
+    #[should_panic(expected = "BadOrigin")]
+    fn origin_parsing_disallows_invalid_origins() {
+        let url = "invalid_url";
+        let _ = Origin::from_str(url).unwrap();
+    }
+
+    #[test]
+    fn origin_parses_opaque_origins() {
+        let url = "blob://foobar";
+        let parsed = not_err!(Origin::from_str(url));
+
+        assert!(!parsed.is_tuple());
+    }
+
     // The following tests check that CORS Request headers are parsed correctly
 
     #[test]

src/lib.rs

@@ -309,8 +309,8 @@ pub enum Error {
     MissingOrigin,
     /// The HTTP request header `Origin` could not be parsed correctly.
     BadOrigin(url::ParseError),
-    /// The configured Allowed Origin is opaque and cannot be parsed.
+    /// The configured Allowed Origins are Opaque origins. Use a Regex instead.
-    OpaqueAllowedOrigin(String),
+    OpaqueAllowedOrigin(Vec<String>),
     /// The request header `Access-Control-Request-Method` is required but is missing
     MissingRequestMethod,
     /// The request header `Access-Control-Request-Method` has an invalid value
@@ -399,11 +399,11 @@ impl fmt::Display for Error {
                 "The `on_response` handler of Fairing could not find the injected header from the \
                  Request. Either some other fairing has removed it, or this is a bug.")
             }
-            Error::OpaqueAllowedOrigin(ref origin) => write!(
+            Error::OpaqueAllowedOrigin(ref origins) => write!(
                 f,
-                "The configured Origin '{}' does \
+                "The configured Origins '{}' are Opaque Origins. \
-                 not have a parsable Origin. Use a regex instead.",
+                 Use regex instead.",
-                origin
+                origins.join("; ")
             ),
             Error::RegexError(ref e) => write!(f, "{}", e),
         }
@@ -462,7 +462,7 @@ impl<T> Default for AllOrSome<T> {
 impl<T> AllOrSome<T> {
     /// Returns whether this is an `All` variant
     pub fn is_all(&self) -> bool {
-        match *self {
+        match self {
             AllOrSome::All => true,
             AllOrSome::Some(_) => false,
         }
@@ -472,6 +472,17 @@ impl<T> AllOrSome<T> {
     pub fn is_some(&self) -> bool {
         !self.is_all()
     }
+
+    /// Unwrap a `Some` variant and get its inner value
+    ///
+    /// # Panics
+    /// Panics if the variant is `All`
+    pub fn unwrap(self) -> T {
+        match self {
+            AllOrSome::All => panic!("Attempting to unwrap an `All`"),
+            AllOrSome::Some(inner) => inner,
+        }
+    }
 }
 
 /// A wrapper type around `rocket::http::Method` to support serialization and deserialization
@@ -566,6 +577,15 @@ mod method_serde {
 /// [ASCII Serialization](https://html.spec.whatwg.org/multipage/origin.html#ascii-serialisation-of-an-origin)
 /// of the origin.
 ///
+/// # Opaque Origins
+/// The [specification](https://html.spec.whatwg.org/multipage/origin.html) defines an Opaque Origin
+/// as one that cannot be recreated. You can refer to the source code for the [`url::Url::origin`]
+/// method to see how an Opaque Origin is determined. Examples of Opaque origins might include
+/// schemes like `file://` or Browser specific schemes like `"moz-extension://` or
+/// `chrome-extension://`.
+///
+/// Opaque Origins cannot be matched exactly. You must use Regex to match Opaque Origins. If you
+/// attempt to create [`Cors`] from [`CorsOptions`], you will get an error.
 /// # Warning about Regex expressions
 /// By default, regex expressions are
 /// [unanchored](https://docs.rs/regex/1.1.2/regex/struct.RegexSet.html#method.is_match).
@@ -603,6 +623,15 @@ impl AllowedOrigins {
     /// [ASCII Serialization](https://html.spec.whatwg.org/multipage/origin.html#ascii-serialisation-of-an-origin)
     /// of the origin.
     ///
+    /// # Opaque Origins
+    /// The [specification](https://html.spec.whatwg.org/multipage/origin.html) defines an Opaque Origin
+    /// as one that cannot be recreated. You can refer to the source code for the [`url::Url::origin`]
+    /// method to see how an Opaque Origin is determined. Examples of Opaque origins might include
+    /// schemes like `file://` or Browser specific schemes like `"moz-extension://` or
+    /// `chrome-extension://`.
+    ///
+    /// Opaque Origins cannot be matched exactly. You must use Regex to match Opaque Origins. If you
+    /// attempt to create [`Cors`] from [`CorsOptions`], you will get an error.
     /// # Warning about Regex expressions
     /// By default, regex expressions are
     /// [unanchored](https://docs.rs/regex/1.1.2/regex/struct.RegexSet.html#method.is_match).
@@ -626,6 +655,12 @@ impl AllowedOrigins {
     /// Exact matches are matched exactly with the
     /// [ASCII Serialization](https://html.spec.whatwg.org/multipage/origin.html#ascii-serialisation-of-an-origin)
     /// of the origin.
+    /// # Opaque Origins
+    /// The [specification](https://html.spec.whatwg.org/multipage/origin.html) defines an Opaque Origin
+    /// as one that cannot be recreated. You can refer to the source code for the [`url::Url::origin`]
+    /// method to see how an Opaque Origin is determined. Examples of Opaque origins might include
+    /// schemes like `file://` or Browser specific schemes like `"moz-extension://` or
+    /// `chrome-extension://`.
     pub fn some_exact<S: AsRef<str>>(exact: &[S]) -> Self {
         AllOrSome::Some(Origins {
             exact: Some(exact.iter().map(|s| s.as_ref().to_string()).collect()),
@@ -687,6 +722,16 @@ impl AllowedOrigins {
 /// [ASCII Serialization](https://html.spec.whatwg.org/multipage/origin.html#ascii-serialisation-of-an-origin)
 /// of the origin.
 ///
+/// # Opaque Origins
+/// The [specification](https://html.spec.whatwg.org/multipage/origin.html) defines an Opaque Origin
+/// as one that cannot be recreated. You can refer to the source code for the [`url::Url::origin`]
+/// method to see how an Opaque Origin is determined. Examples of Opaque origins might include
+/// schemes like `file://` or Browser specific schemes like `"moz-extension://` or
+/// `chrome-extension://`.
+///
+/// Opaque Origins cannot be matched exactly. You must use Regex to match Opaque Origins. If you
+/// attempt to create [`Cors`] from [`CorsOptions`], you will get an error.
+///
 /// # Warning about Regex expressions
 /// By default, regex expressions are
 /// [unanchored](https://docs.rs/regex/1.1.2/regex/struct.RegexSet.html#method.is_match).
@@ -709,6 +754,16 @@ pub struct Origins {
     /// Exact matches are matched exactly with the
     /// [ASCII Serialization](https://html.spec.whatwg.org/multipage/origin.html#ascii-serialisation-of-an-origin)
     /// of the origin.
+    ///
+    /// # Opaque Origins
+    /// The [specification](https://html.spec.whatwg.org/multipage/origin.html) defines an Opaque Origin
+    /// as one that cannot be recreated. You can refer to the source code for the [`url::Url::origin`]
+    /// method to see how an Opaque Origin is determined. Examples of Opaque origins might include
+    /// schemes like `file://` or Browser specific schemes like `"moz-extension://` or
+    /// `chrome-extension://`.
+    ///
+    /// Opaque Origins cannot be matched exactly. You must use Regex to match Opaque Origins. If you
+    /// attempt to create [`Cors`] from [`CorsOptions`], you will get an error.
     #[cfg_attr(feature = "serialization", serde(default))]
     pub exact: Option<HashSet<String>>,
     /// Origins that will be matched via __any__ regex in this list.
@@ -743,20 +798,29 @@ pub(crate) struct ParsedAllowedOrigins {
 
 impl ParsedAllowedOrigins {
     fn parse(origins: &Origins) -> Result<Self, Error> {
-        let exact: Result<HashSet<url::Origin>, Error> = match &origins.exact {
+        let exact: Result<Vec<(&str, url::Origin)>, Error> = match &origins.exact {
-            Some(exact) => exact.iter().map(|url| to_origin(url.as_str())).collect(),
+            Some(exact) => exact
+                .iter()
+                .map(|url| Ok((url.as_str(), to_origin(url.as_str())?)))
+                .collect(),
             None => Ok(Default::default()),
         };
         let exact = exact?;
 
-        // Let's check if any of them is Opaque
+        // Let's check if they are Opaque
-        exact.iter().try_for_each(|url| {
+        let (tuple, opaque): (Vec<_>, Vec<_>) =
-            if !url.is_tuple() {
+            exact.into_iter().partition(|(_, url)| url.is_tuple());
-                Err(Error::OpaqueAllowedOrigin(url.ascii_serialization()))
+
-            } else {
+        if !opaque.is_empty() {
-                Ok(())
+            Err(Error::OpaqueAllowedOrigin(
+                opaque
+                    .into_iter()
+                    .map(|(original, _)| original.to_string())
+                    .collect(),
+            ))?
         }
-        })?;
+
+        let exact = tuple.into_iter().map(|(_, url)| url).collect();
 
         let regex = match &origins.regex {
             None => None,
@@ -771,6 +835,7 @@ impl ParsedAllowedOrigins {
     }
 
     fn verify(&self, origin: &Origin) -> bool {
+        info_!("Verifying origin: {}", origin);
         match origin {
             Origin::Null => {
                 info_!("Origin is null. Allowing? {}", self.allow_null);
@@ -1945,39 +2010,6 @@ mod tests {
         Client::new(rocket).expect("valid rocket instance")
     }
 
-    // `to_origin` tests
-
-    #[test]
-    fn origin_is_parsed_properly() {
-        let url = "https://foo.bar.xyz";
-        let parsed = not_err!(Origin::from_str(url));
-        assert_eq!(parsed.ascii_serialization(), url);
-    }
-
-    #[test]
-    fn origin_parsing_strips_paths() {
-        // this should never really be sent by a compliant user agent
-        let url = "https://foo.bar.xyz/path/somewhere";
-        let parsed = not_err!(Origin::from_str(url));
-        let expected = "https://foo.bar.xyz";
-        assert_eq!(parsed.ascii_serialization(), expected);
-    }
-
-    #[test]
-    #[should_panic(expected = "BadOrigin")]
-    fn origin_parsing_disallows_invalid_origins() {
-        let url = "invalid_url";
-        let _ = Origin::from_str(url).unwrap();
-    }
-
-    #[test]
-    fn origin_parses_opaque_origins() {
-        let url = "blob://foobar";
-        let parsed = not_err!(Origin::from_str(url));
-
-        assert!(!parsed.is_tuple());
-    }
-
     // CORS options test
 
     #[test]
@@ -2073,6 +2105,54 @@ mod tests {
         let _ = AllowedOrigins::some(&static_exact, &random_regex);
     }
 
+    // `ParsedAllowedOrigins::parse` tests
+    #[test]
+    fn allowed_origins_are_parsed_correctly() {
+        let allowed_origins = not_err!(parse_allowed_origins(&AllowedOrigins::some(
+            &["https://www.acme.com"],
+            &["^https://www.example-[A-z0-9]+.com$"]
+        )));
+        assert!(allowed_origins.is_some());
+
+        let expected_exact: HashSet<url::Origin> = [url::Url::from_str("https://www.acme.com")
+            .expect("not to fail")
+            .origin()]
+        .iter()
+        .map(Clone::clone)
+        .collect();
+        let expected_regex = ["^https://www.example-[A-z0-9]+.com$"];
+
+        let actual = allowed_origins.unwrap();
+        assert_eq!(expected_exact, actual.exact);
+        assert_eq!(expected_regex, actual.regex.expect("to be some").patterns());
+    }
+
+    #[test]
+    fn allowed_origins_errors_on_opaque_exact() {
+        let error = parse_allowed_origins(&AllowedOrigins::some::<_, &str>(
+            &[
+                "chrome-extension://something",
+                "moz-extension://something",
+                "https://valid.com",
+            ],
+            &[],
+        ))
+        .unwrap_err();
+
+        match error {
+            Error::OpaqueAllowedOrigin(mut origins) => {
+                origins.sort();
+                assert_eq!(
+                    origins,
+                    ["chrome-extension://something", "moz-extension://something"]
+                );
+            }
+            others => {
+                panic!("Unexpected error: {:#?}", others);
+            }
+        };
+    }
+
     // The following tests check validation
 
     #[test]
@@ -2135,7 +2215,6 @@ mod tests {
     fn validate_origin_validates_opaque_origins() {
         let url = "moz-extension://8c7c4444-e29f-…cb8-1ade813dbd12/js/content.js:505";
         let origin = not_err!(to_parsed_origin(&url));
-        println!("{:#?}", origin);
         let allowed_origins = not_err!(parse_allowed_origins(&AllowedOrigins::some_regex(&[
             "moz-extension://.*"
         ])));