fix(router): correct over-encoding of URL fragment (#22687)

jasonaden · kara · commit 0bf6fa5b324b · 2018-03-11T22:15:01.000-07:00
Relates to: #10280 #22337 PR Close #22687
diff --git a/packages/router/src/url_tree.ts b/packages/router/src/url_tree.ts
@@ -280,7 +280,8 @@ export class DefaultUrlSerializer implements UrlSerializer {
   serialize(tree: UrlTree): string {
     const segment = `/${serializeSegment(tree.root, true)}`;
     const query = serializeQueryParams(tree.queryParams);
-    const fragment = typeof tree.fragment === `string` ? `#${encodeUriQuery(tree.fragment !)}` : '';
+    const fragment =
+        typeof tree.fragment === `string` ? `#${encodeUriFragment(tree.fragment !)}` : '';
 
     return `${segment}${query}${fragment}`;
   }
@@ -329,13 +330,7 @@ function serializeSegment(segment: UrlSegmentGroup, root: boolean): string {
  * Encodes a URI string with the default encoding. This function will only ever be called from
  * `encodeUriQuery` or `encodeUriSegment` as it's the base set of encodings to be used. We need
  * a custom encoding because encodeURIComponent is too aggressive and encodes stuff that doesn't
- * have to be encoded per http://tools.ietf.org/html/rfc3986:
- *    query         = *( pchar / "/" / "?" )
- *    pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
- *    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
- *    pct-encoded   = "%" HEXDIG HEXDIG
- *    sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
- *                     / "*" / "+" / "," / ";" / "="
+ * have to be encoded per https://url.spec.whatwg.org.
  */
 function encodeUriString(s: string): string {
   return encodeURIComponent(s)
@@ -346,15 +341,25 @@ function encodeUriString(s: string): string {
 }
 
 /**
- * This function should be used to encode both keys and values in a query string key/value or the
- * URL fragment. In the following URL, you need to call encodeUriQuery on "k", "v" and "f":
+ * This function should be used to encode both keys and values in a query string key/value. In
+ * the following URL, you need to call encodeUriQuery on "k" and "v":
  *
  * http://www.site.org/html;mk=mv?k=v#f
  */
 export function encodeUriQuery(s: string): string {
   return encodeUriString(s).replace(/%3B/gi, ';');
 }
 
+/**
+ * This function should be used to encode a URL fragment. In the following URL, you need to call
+ * encodeUriFragment on "f":
+ *
+ * http://www.site.org/html;mk=mv?k=v#f
+ */
+export function encodeUriFragment(s: string): string {
+  return encodeURI(s);
+}
+
 /**
  * This function should be run on any URI segment as well as the key and value in a key/value
  * pair for matrix params. In the following URL, you need to call encodeUriSegment on "html",
diff --git a/packages/router/test/url_serializer.spec.ts b/packages/router/test/url_serializer.spec.ts
@@ -7,7 +7,7 @@
  */
 
 import {PRIMARY_OUTLET} from '../src/shared';
-import {DefaultUrlSerializer, UrlSegmentGroup, encodeUriQuery, encodeUriSegment, serializePath} from '../src/url_tree';
+import {DefaultUrlSerializer, UrlSegmentGroup, encodeUriFragment, encodeUriQuery, encodeUriSegment, serializePath} from '../src/url_tree';
 
 describe('url serializer', () => {
   const url = new DefaultUrlSerializer();
@@ -254,11 +254,11 @@ describe('url serializer', () => {
     });
 
     it('should encode/decode fragment', () => {
-      const u = `/one#${encodeUriQuery('one two=three four')}`;
+      const u = `/one#${encodeUriFragment('one two=three four')}`;
       const tree = url.parse(u);
 
       expect(tree.fragment).toEqual('one two=three four');
-      expect(url.serialize(tree)).toEqual('/one#one%20two%3Dthree%20four');
+      expect(url.serialize(tree)).toEqual('/one#one%20two=three%20four');
     });
   });
 
@@ -311,7 +311,7 @@ describe('url serializer', () => {
     // From http://www.ietf.org/rfc/rfc3986.txt
     const unreserved = `abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._~`;
 
-    it('should encode a minimal set of special characters in queryParams and fragment', () => {
+    it('should encode a minimal set of special characters in queryParams', () => {
       const notEncoded = unreserved + `:@!$'*,();`;
       const encode = ` +%&=#[]/?`;
       const encoded = `%20%2B%25%26%3D%23%5B%5D%2F%3F`;
@@ -324,9 +324,9 @@ describe('url serializer', () => {
     });
 
     it('should encode a minimal set of special characters in fragment', () => {
-      const notEncoded = unreserved + `:@!$'*,();`;
-      const encode = ` +%&=#[]/?`;
-      const encoded = `%20%2B%25%26%3D%23%5B%5D%2F%3F`;
+      const notEncoded = unreserved + `:@!$'*,();+&=#/?`;
+      const encode = ' %<>`"[]';
+      const encoded = `%20%25%3C%3E%60%22%5B%5D`;
 
       const parsed = url.parse('/foo');
 
