fixup! fix(router): fix URL serialization so special characters are o…

…nly encoded where needed
angular · Feb 21, 2018 · 6c6af62 · 6c6af62
1 parent 5431d29
commit 6c6af62
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 20 deletions.
diff --git a/packages/router/src/url_tree.ts b/packages/router/src/url_tree.ts
@@ -338,26 +338,30 @@ function serializeSegment(segment: UrlSegmentGroup, root: boolean): string {
  *                     / "*" / "+" / "," / ";" / "="
  */
 function encodeUriString(s: string, type: 'query' | 'segment' = 'query'): string {
-  let encoded = encodeURIComponent(s)
-                    .replace(/%40/g, '@')
-                    .replace(/%3A/gi, ':')
-                    .replace(/%24/g, '$')
-                    .replace(/%2C/gi, ',');
-
-  return encoded;
+  return encodeURIComponent(s)
+      .replace(/%40/g, '@')
+      .replace(/%3A/gi, ':')
+      .replace(/%24/g, '$')
+      .replace(/%2C/gi, ',');
 }
 
 /**
- * Encodes a string for use in the query string or fragment. This function should be used to
- * encode both keys and values in a query string key/value pair as well as the fragment string.
+ * This function should be used to encode both keys and values in a query string key/value
+ * pair as well as the fragment string. In the following URL, you need to call
+ * encodeUriQuery on "k", "v", and "f":
+ *
+ * http://www.site.org/html;mk=mv?k=v#f
  */
 export function encodeUriQuery(s: string): string {
   return encodeUriString(s).replace(/%3B/gi, ';');
 }
 
 /**
- * Encodes for the URI segment. This is the path portion of a URI, before the query string
- * and/or fragment.
+ * 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",
+ * "mk", and "mv":
+ *
+ * http://www.site.org/html;mk=mv?k=v#f
  */
 export function encodeUriSegment(s: string): string {
   return encodeUriString(s)
@@ -372,10 +376,10 @@ export function decode(s: string): string {
 }
 
 export function serializePath(path: UrlSegment): string {
-  return `${encodeUriSegment(path.path)}${serializeParams(path.parameters)}`;
+  return `${encodeUriSegment(path.path)}${serializeMatrixParams(path.parameters)}`;
 }
 
-function serializeParams(params: {[key: string]: string}): string {
+function serializeMatrixParams(params: {[key: string]: string}): string {
   return Object.keys(params)
       .map(key => `;${encodeUriSegment(key)}=${encodeUriSegment(params[key])}`)
       .join('');

diff --git a/packages/router/test/url_serializer.spec.ts b/packages/router/test/url_serializer.spec.ts
@@ -189,7 +189,7 @@ describe('url serializer', () => {
   describe('encoding/decoding', () => {
     it('should encode/decode path segments and parameters', () => {
       const u =
-          `/${encodeUriQuery("one two")};${encodeUriQuery("p 1")}=${encodeUriQuery("v 1")};${encodeUriQuery("p 2")}=${encodeUriQuery("v 2")}`;
+          `/${encodeUriSegment("one two")};${encodeUriSegment("p 1")}=${encodeUriSegment("v 1")};${encodeUriSegment("p 2")}=${encodeUriSegment("v 2")}`;
       const tree = url.parse(u);
 
       expect(tree.root.children[PRIMARY_OUTLET].segments[0].path).toEqual('one two');
@@ -250,8 +250,8 @@ describe('url serializer', () => {
 
     // Tests specific to https://github.com/angular/angular/issues/10280
     it('should parse encoded parens in matrix params', () => {
-      const auxRoutesUrl = '/abc;foo=(aux:one)';
-      const fooValueUrl = '/abc;foo=%28one%29';
+      const auxRoutesUrl = '/abc;foo=(other:val)';
+      const fooValueUrl = '/abc;foo=%28other:val%29';
 
       const auxParsed = url.parse(auxRoutesUrl).root;
       const fooParsed = url.parse(fooValueUrl).root;
@@ -261,13 +261,13 @@ describe('url serializer', () => {
       expect(auxParsed.children[PRIMARY_OUTLET].segments.length).toBe(1);
       expect(auxParsed.children[PRIMARY_OUTLET].segments[0].path).toBe('abc');
       expect(auxParsed.children[PRIMARY_OUTLET].segments[0].parameters).toEqual({foo: ''});
-      expect(auxParsed.children['aux'].segments.length).toBe(1);
-      expect(auxParsed.children['aux'].segments[0].path).toBe('one');
+      expect(auxParsed.children['other'].segments.length).toBe(1);
+      expect(auxParsed.children['other'].segments[0].path).toBe('val');
 
       // Confirm matrix params are URL decoded
       expect(fooParsed.children[PRIMARY_OUTLET].segments.length).toBe(1);
       expect(fooParsed.children[PRIMARY_OUTLET].segments[0].path).toBe('abc');
-      expect(fooParsed.children[PRIMARY_OUTLET].segments[0].parameters).toEqual({foo: '(one)'});
+      expect(fooParsed.children[PRIMARY_OUTLET].segments[0].parameters).toEqual({foo: '(other:val)'});
     });
 
     it('should serialize encoded parens in matrix params', () => {
@@ -278,7 +278,7 @@ describe('url serializer', () => {
       expect(url.serialize(parsed)).toBe('/abc;foo=%28one%29');
     });
 
-    it('should not serialize encoded parens in url params', () => {
+    it('should not serialize encoded parens in query params', () => {
       const testUrl = '/abc?foo=%28one%29';
 
       const parsed = url.parse(testUrl);