Merge pull request Kixunil#3 from nothingmuch/escaping-rules

Fix escaping of '#' in parameter values and handling of unescaped '#' in uri

src/de.rs

@@ -42,6 +42,16 @@ impl<'a, T: DeserializeParams<'a>> Uri<'a, bitcoin::address::NetworkUnchecked, T
         let mut label = None;
         let mut message = None;
         if let Some(params) = params {
+            // [RFC 3986 § 3.4](https://www.rfc-editor.org/rfc/rfc3986#section-3.4):
+            //
+            // > The query component is indicated by the first question
+            // > mark ("?") character and terminated by a number sign ("#") character
+            // > or by the end of the URI.
+            let params = match params.find('#') {
+                Some(pos) => &params[..pos],
+                None => params,
+            };
+
             for param in params.split('&') {
                 let pos = param
                     .find('=')

src/lib.rs

@@ -426,5 +426,57 @@ mod tests {
         assert!(uri.amount.is_none());
         assert!(uri.label.is_none());
         assert!(uri.message.is_none());
+
+        assert_eq!(uri.to_string(), "bitcoin:1andreas3batLhQa2FawWjeyjCqyBzypd");
+    }
+
+    #[test]
+    fn label_with_rfc3986_param_separator() {
+        let input = "bitcoin:1andreas3batLhQa2FawWjeyjCqyBzypd?label=foo%26bar%20%3D%20baz/blah?;:@";
+        let uri = input.parse::<Uri<'_, _>>().unwrap().require_network(bitcoin::Network::Bitcoin).unwrap();
+        let label: Cow<'_, str> = uri.label.clone().unwrap().try_into().unwrap();
+        assert_eq!(uri.address.to_string(), "1andreas3batLhQa2FawWjeyjCqyBzypd");
+        assert_eq!(label, "foo&bar = baz/blah?;:@");
+        assert!(uri.amount.is_none());
+        assert!(uri.message.is_none());
+
+        assert_eq!(uri.to_string(), input);
+    }
+
+    #[test]
+    fn label_with_rfc3986_fragment_separator() {
+        let input = "bitcoin:1andreas3batLhQa2FawWjeyjCqyBzypd?label=foo%23bar";
+        let uri = input.parse::<Uri<'_, _>>().unwrap().require_network(bitcoin::Network::Bitcoin).unwrap();
+        let label: Cow<'_, str> = uri.label.clone().unwrap().try_into().unwrap();
+        assert_eq!(uri.address.to_string(), "1andreas3batLhQa2FawWjeyjCqyBzypd");
+        assert_eq!(label, "foo#bar");
+        assert!(uri.amount.is_none());
+        assert!(uri.message.is_none());
+
+        assert_eq!(uri.to_string(), input);
+    }
+
+    #[test]
+    fn rfc3986_empty_fragment_not_defined_in_bip21() {
+        let input = "bitcoin:1andreas3batLhQa2FawWjeyjCqyBzypd?label=foo#";
+        let uri = input.parse::<Uri<'_, _>>().unwrap().require_network(bitcoin::Network::Bitcoin).unwrap();
+        let label: Cow<'_, str> = uri.label.clone().unwrap().try_into().unwrap();
+        assert_eq!(uri.address.to_string(), "1andreas3batLhQa2FawWjeyjCqyBzypd");
+        assert_eq!(label, "foo");
+        assert!(uri.amount.is_none());
+        assert!(uri.message.is_none());
+        assert_eq!(uri.to_string(), "bitcoin:1andreas3batLhQa2FawWjeyjCqyBzypd?label=foo");
+    }
+
+    #[test]
+    fn rfc3986_non_empty_fragment_not_defined_in_bip21() {
+        let input = "bitcoin:1andreas3batLhQa2FawWjeyjCqyBzypd?label=foo#&message=not%20part%20of%20a%20message";
+        let uri = input.parse::<Uri<'_, _>>().unwrap().require_network(bitcoin::Network::Bitcoin).unwrap();
+        let label: Cow<'_, str> = uri.label.clone().unwrap().try_into().unwrap();
+        assert_eq!(uri.address.to_string(), "1andreas3batLhQa2FawWjeyjCqyBzypd");
+        assert_eq!(label, "foo");
+        assert!(uri.amount.is_none());
+        assert!(uri.message.is_none());
+        assert_eq!(uri.to_string(), "bitcoin:1andreas3batLhQa2FawWjeyjCqyBzypd?label=foo");
     }
 }

src/ser.rs

@@ -47,7 +47,61 @@ impl<'a, W: fmt::Write> fmt::Write for EqSignChecker<'a, W> {
 }
 
 /// Set of characters that will be percent-encoded
-const ASCII_SET: percent_encoding_rfc3986::AsciiSet = percent_encoding_rfc3986::CONTROLS.add(b'&').add(b'?').add(b' ').add(b'=');
+///
+/// This contains anything not in `query` (i.e. ``gen-delim` from the quoted
+/// definitions`) as per RFC 3986, as well as '&' and '=' as per BIP 21.
+///
+/// [BIP 21](https://github.com/bitcoin/bips/blob/master/bip-0021.mediawiki#abnf-grammar):
+///
+/// > ```text
+/// > labelparam     = "label=" *qchar
+/// > messageparam   = "message=" *qchar
+/// > otherparam     = qchar *qchar [ "=" *qchar ]
+/// > ```
+/// ...
+/// > Here, "qchar" corresponds to valid characters of an RFC 3986 URI query
+/// > component, excluding the "=" and "&" characters, which this BIP takes as
+/// > separators.
+///
+/// [RFC 3986 Appendix A](https://www.rfc-editor.org/rfc/rfc3986#appendix-A):
+///
+/// > ```text
+/// > pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
+/// > query         = *( pchar / "/" / "?" )
+/// > ```
+/// ...
+/// > ```text
+/// > pct-encoded   = "%" HEXDIG HEXDIG
+/// > unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
+/// > ```
+/// ...
+/// > ```text
+/// > sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
+/// >               / "*" / "+" / "," / ";" / "="
+/// > ```
+const ASCII_SET: percent_encoding_rfc3986::AsciiSet = percent_encoding_rfc3986::NON_ALPHANUMERIC
+    // allow non-alphanumeric characters from `unreserved`
+    .remove(b'-')
+    .remove(b'.')
+    .remove(b'_')
+    .remove(b'~')
+    // allow non-alphanumeric characters from `sub-delims` excluding bip-21
+    // separators ("&", and "=")
+    .remove(b'!')
+    .remove(b'$')
+    .remove(b'\'')
+    .remove(b'(')
+    .remove(b')')
+    .remove(b'*')
+    .remove(b'+')
+    .remove(b',')
+    .remove(b';')
+    // allow pchar extra chars
+    .remove(b':')
+    .remove(b'@')
+    // allow query extra chars
+    .remove(b'/')
+    .remove(b'?');
 
 /// Percent-encodes writes.
 struct WriterEncoder<W: fmt::Write>(W);