Skip to content

Commit

Permalink
fix(router): correct over-encoding of URL fragment (#22687)
Browse files Browse the repository at this point in the history
Relates to: #10280 #22337

PR Close #22687
  • Loading branch information
jasonaden authored and kara committed Mar 12, 2018
1 parent 6d9a4f8 commit 86517f2
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 17 deletions.
25 changes: 15 additions & 10 deletions packages/router/src/url_tree.ts
Expand Up @@ -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}`;
}
Expand Down Expand Up @@ -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)
Expand All @@ -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",
Expand Down
14 changes: 7 additions & 7 deletions packages/router/test/url_serializer.spec.ts
Expand Up @@ -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();
Expand Down Expand Up @@ -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');
});
});

Expand Down Expand Up @@ -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`;
Expand All @@ -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');

Expand Down

0 comments on commit 86517f2

Please sign in to comment.