Skip to content

Commit

Permalink
fix(router): fix URL serialization so special characters are only enc…
Browse files Browse the repository at this point in the history 
…oded where needed

Fixes: #10280
  • Loading branch information
@jasonaden
jasonaden committed Feb 21, 2018
1 parent 8115edc commit dc0de4a
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 22 deletions.
27 changes: 18 additions & 9 deletions packages/router/src/url_tree.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ 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` ? `#${encodeURI(tree.fragment !)}` : '';
const fragment = typeof tree.fragment === `string` ? `#${encodeUriParams(tree.fragment !)}` : '';

return `${segment}${query}${fragment}`;
}
Expand Down Expand Up @@ -336,32 +336,41 @@ function serializeSegment(segment: UrlSegmentGroup, root: boolean): string {
* sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
* / "*" / "+" / "," / ";" / "="
*/
export function encode(s: string): string {
return encodeURIComponent(s)
export function encodeUriParams(s: string, type: 'query' | 'segment' = 'query'): string {
let encoded = encodeURIComponent(s)
.replace(/%40/g, '@')
.replace(/%3A/gi, ':')
.replace(/%24/g, '$')
.replace(/%2C/gi, ',')
.replace(/%3B/gi, ';');
.replace(/%5B/gi, '[')
.replace(/%5D/gi, ']');

encoded = type === 'segment' ?
encoded.replace(/\(/g, '%28').replace(/\)/g, '%29') :
encoded.replace(/%3B/gi, ';').replace(/%3F/gi, '?').replace(/%2F/gi, '/');

return encoded;
}

export function decode(s: string): string {
return decodeURIComponent(s);
}

export function serializePath(path: UrlSegment): string {
return `${encode(path.path)}${serializeParams(path.parameters)}`;
return `${encodeUriParams(path.path, 'segment')}${serializeParams(path.parameters)}`;
}

function serializeParams(params: {[key: string]: string}): string {
return Object.keys(params).map(key => `;${encode(key)}=${encode(params[key])}`).join('');
return Object.keys(params)
.map(key => `;${encodeUriParams(key, 'segment')}=${encodeUriParams(params[key], 'segment')}`)
.join('');
}

function serializeQueryParams(params: {[key: string]: any}): string {
const strParams: string[] = Object.keys(params).map((name) => {
const value = params[name];
return Array.isArray(value) ? value.map(v => `${encode(name)}=${encode(v)}`).join('&') :
`${encode(name)}=${encode(value)}`;
return Array.isArray(value) ? value.map(v => `${encodeUriParams(name)}=${encodeUriParams(v)}`).join('&') :
`${encodeUriParams(name)}=${encodeUriParams(value)}`;
});

return strParams.length ? `?${strParams.join("&")}` : '';
Expand Down Expand Up @@ -414,7 +423,7 @@ class UrlParser {
}

parseFragment(): string|null {
return this.consumeOptional('#') ? decodeURI(this.remaining) : null;
return this.consumeOptional('#') ? decodeURIComponent(this.remaining) : null;
}

private parseChildren(): {[outlet: string]: UrlSegmentGroup} {
Expand Down
114 changes: 101 additions & 13 deletions packages/router/test/url_serializer.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {PRIMARY_OUTLET} from '../src/shared';
import {DefaultUrlSerializer, UrlSegmentGroup, encode, serializePath} from '../src/url_tree';
import {DefaultUrlSerializer, UrlSegmentGroup, encodeUriParams, serializePath} from '../src/url_tree';

describe('url serializer', () => {
const url = new DefaultUrlSerializer();
Expand Down Expand Up @@ -189,7 +189,7 @@ describe('url serializer', () => {
describe('encoding/decoding', () => {
it('should encode/decode path segments and parameters', () => {
const u =
`/${encode("one two")};${encode("p 1")}=${encode("v 1")};${encode("p 2")}=${encode("v 2")}`;
`/${encodeUriParams("one two")};${encodeUriParams("p 1")}=${encodeUriParams("v 1")};${encodeUriParams("p 2")}=${encodeUriParams("v 2")}`;
const tree = url.parse(u);

expect(tree.root.children[PRIMARY_OUTLET].segments[0].path).toEqual('one two');
Expand All @@ -199,7 +199,7 @@ describe('url serializer', () => {
});

it('should encode/decode "slash" in path segments and parameters', () => {
const u = `/${encode("one/two")};${encode("p/1")}=${encode("v/1")}/three`;
const u = `/${encodeUriParams("one/two", 'segment')};${encodeUriParams("p/1", 'segment')}=${encodeUriParams("v/1", 'segment')}/three`;
const tree = url.parse(u);
const segment = tree.root.children[PRIMARY_OUTLET].segments[0];
expect(segment.path).toEqual('one/two');
Expand All @@ -210,7 +210,7 @@ describe('url serializer', () => {
});

it('should encode/decode query params', () => {
const u = `/one?${encode("p 1")}=${encode("v 1")}&${encode("p 2")}=${encode("v 2")}`;
const u = `/one?${encodeUriParams("p 1")}=${encodeUriParams("v 1")}&${encodeUriParams("p 2")}=${encodeUriParams("v 2")}`;
const tree = url.parse(u);

expect(tree.queryParams).toEqual({'p 1': 'v 1', 'p 2': 'v 2'});
Expand All @@ -220,27 +220,115 @@ describe('url serializer', () => {
});

it('should encode query params leaving sub-delimiters intact', () => {
const percentChars = '/?#[]&+= ';
const percentCharsEncoded = '%2F%3F%23%5B%5D%26%2B%3D%20';
const intactChars = '!$\'()*,;:';
const percentChars = '#&+= ';
const percentCharsEncoded = '%23%26%2B%3D%20';
const intactChars = '/?!$\'[]()*,;:';
const params = percentChars + intactChars;
const paramsEncoded = percentCharsEncoded + intactChars;
const mixedCaseString = 'sTrInG';

expect(percentCharsEncoded).toEqual(encode(percentChars));
expect(intactChars).toEqual(encode(intactChars));
expect(percentCharsEncoded).toEqual(encodeUriParams(percentChars));
expect(intactChars).toEqual(encodeUriParams(intactChars));
// Verify it replaces repeated characters correctly
expect(paramsEncoded + paramsEncoded).toEqual(encode(params + params));
expect(paramsEncoded + paramsEncoded).toEqual(encodeUriParams(params + params));
// Verify it doesn't change the case of alpha characters
expect(mixedCaseString + paramsEncoded).toEqual(encode(mixedCaseString + params));
expect(mixedCaseString + paramsEncoded).toEqual(encodeUriParams(mixedCaseString + params));
});

it('should encode/decode fragment', () => {
const u = `/one#${encodeURI("one two=three four")}`;
const u = `/one#${encodeUriParams('one two=three four')}`;
const tree = url.parse(u);

expect(tree.fragment).toEqual('one two=three four');
expect(url.serialize(tree)).toEqual(u);
expect(url.serialize(tree)).toEqual('/one#one%20two%3Dthree%20four');
});
});

describe('special character encoding/decoding', () => {
const pathSpecialCharacters = `"'()[]*#@`
const paramSpecialCharacters = `"'()[]*#@`

// 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 auxParsed = url.parse(auxRoutesUrl).root;
const fooParsed = url.parse(fooValueUrl).root;


// Test base case
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');

// 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)'});
});

it('should serialize encoded parens in matrix params', () => {
const testUrl = '/abc;foo=%28one%29';

const parsed = url.parse(testUrl);

expect(url.serialize(parsed)).toBe('/abc;foo=%28one%29');
});

it('should not serialize encoded parens in url params', () => {
const testUrl = '/abc?foo=%28one%29';

const parsed = url.parse(testUrl);

expect(parsed.queryParams).toEqual({foo: '(one)'});

expect(url.serialize(parsed)).toBe('/abc?foo=(one)');
});

// Test special characters in general

// From http://www.ietf.org/rfc/rfc3986.txt
const unreserved = `abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._~`;

it('should encode a minimal set of special characters in queryParams & fragment', () => {
const notEncoded = unreserved + `:/?[]@!$'*,();`;
const encode = ` +%&=#`;
const encoded = `%20%2B%25%26%3D%23`;

const parsed = url.parse('/foo');

parsed.queryParams = {notEncoded, encode};
parsed.fragment = notEncoded + encode;

expect(url.serialize(parsed)).toBe(`/foo?notEncoded=${notEncoded}&encode=${encoded}#${notEncoded}${encoded}`);
});

it('should encode minimal special characters plus parens and semi-colon in matrix params', () => {
const notEncoded = unreserved + `:[]@!$'*,`;
const encode = ` /+%&=#();?`;
const encoded = `%20%2F%2B%25%26%3D%23%28%29%3B%3F`;

const parsed = url.parse('/foo');

parsed.root.children[PRIMARY_OUTLET].segments[0].parameters = {notEncoded, encode};

expect(url.serialize(parsed))
.toBe(`/foo;notEncoded=${notEncoded};encode=${encoded}`);
});

it('should encode special characters in the path the same as matrix params', () => {
const notEncoded = unreserved + `:[]@!$'*,`;
const encode = ` /+%&=#();?`;
const encoded = `%20%2F%2B%25%26%3D%23%28%29%3B%3F`;

const parsed = url.parse('/foo');

parsed.root.children[PRIMARY_OUTLET].segments[0].path = notEncoded + encode;

expect(url.serialize(parsed)).toBe(`/${notEncoded}${encoded}`);
});
});

Expand Down

0 comments on commit dc0de4a

Please sign in to comment.