From 16204263939944a04e717f4dd44fdb9eb63119c5 Mon Sep 17 00:00:00 2001 From: Jeff Cross Date: Tue, 28 Jun 2016 11:31:35 -0700 Subject: [PATCH] fix(http): don't encode values that are allowed in query (#9651) This implements a new class, QueryEncoder, that provides methods for encoding keys and values of query parameter. The encoder encodes with encodeURIComponent, and then decodes a whitelist of allowed characters back to their unencoded form. BREAKING CHANGE: The changes to Http's URLSearchParams serialization now prevent encoding of these characters inside query parameters which were previously converted to percent-encoded values: @ : $ , ; + ; ? / The default encoding behavior can be overridden by extending QueryEncoder, as documented in the URLSearchParams service. Fixes #9348 --- modules/@angular/http/http.ts | 3 +- .../@angular/http/src/url_search_params.ts | 57 ++++++++++++++++++- .../http/test/url_search_params_spec.ts | 46 ++++++++++++++- tools/public_api_guard/http/index.d.ts | 8 ++- 4 files changed, 108 insertions(+), 6 deletions(-) diff --git a/modules/@angular/http/http.ts b/modules/@angular/http/http.ts index 762ef343d7..5dcd3bb082 100644 --- a/modules/@angular/http/http.ts +++ b/modules/@angular/http/http.ts @@ -34,7 +34,8 @@ export {Http, Jsonp} from './src/http'; export {Connection, ConnectionBackend, RequestOptionsArgs, ResponseOptionsArgs, XSRFStrategy} from './src/interfaces'; export {Request} from './src/static_request'; export {Response} from './src/static_response'; -export {URLSearchParams} from './src/url_search_params'; +export {QueryEncoder, URLSearchParams} from './src/url_search_params'; + /** diff --git a/modules/@angular/http/src/url_search_params.ts b/modules/@angular/http/src/url_search_params.ts index add96e00ec..76484d6370 100644 --- a/modules/@angular/http/src/url_search_params.ts +++ b/modules/@angular/http/src/url_search_params.ts @@ -14,7 +14,7 @@ function paramParser(rawParams: string = ''): Map { if (rawParams.length > 0) { var params: string[] = rawParams.split('&'); params.forEach((param: string) => { - var split: string[] = param.split('='); + var split: string[] = param.split('=', 2); var key = split[0]; var val = split[1]; var list = isPresent(map.get(key)) ? map.get(key) : []; @@ -24,6 +24,27 @@ function paramParser(rawParams: string = ''): Map { } return map; } +/** + * @experimental + **/ +export class QueryEncoder { + encodeKey(k: string): string { return standardEncoding(k); } + + encodeValue(v: string): string { return standardEncoding(v); } +} + +function standardEncoding(v: string): string { + return encodeURIComponent(v) + .replace(/%40/gi, '@') + .replace(/%3A/gi, ':') + .replace(/%24/gi, '$') + .replace(/%2C/gi, ',') + .replace(/%3B/gi, ';') + .replace(/%2B/gi, '+') + .replace(/%3D/gi, ';') + .replace(/%3F/gi, '?') + .replace(/%2F/gi, '/'); +} /** * Map-like representation of url search parameters, based on @@ -33,11 +54,39 @@ function paramParser(rawParams: string = ''): Map { * - appendAll() * - replaceAll() * + * This class accepts an optional second parameter of ${@link QueryEncoder}, + * which is used to serialize parameters before making a request. By default, + * `QueryEncoder` encodes keys and values of parameters using `encodeURIComponent`, + * and then un-encodes certain characters that are allowed to be part of the query + * according to IETF RFC 3986: https://tools.ietf.org/html/rfc3986. + * + * These are the characters that are not encoded: `! $ \' ( ) * + , ; A 9 - . _ ~ ? /` + * + * If the set of allowed query characters is not acceptable for a particular backend, + * `QueryEncoder` can be subclassed and provided as the 2nd argument to URLSearchParams. + * + * ``` + * import {URLSearchParams, QueryEncoder} from '@angular/http'; + * class MyQueryEncoder extends QueryEncoder { + * encodeKey(k: string): string { + * return myEncodingFunction(k); + * } + * + * encodeValue(v: string): string { + * return myEncodingFunction(v); + * } + * } + * + * let params = new URLSearchParams('', new MyQueryEncoder()); + * ``` * @experimental */ export class URLSearchParams { paramsMap: Map; - constructor(public rawParams: string = '') { this.paramsMap = paramParser(rawParams); } + constructor( + public rawParams: string = '', private queryEncoder: QueryEncoder = new QueryEncoder()) { + this.paramsMap = paramParser(rawParams); + } clone(): URLSearchParams { var clone = new URLSearchParams(); @@ -133,7 +182,9 @@ export class URLSearchParams { toString(): string { var paramsList: string[] = []; this.paramsMap.forEach((values, k) => { - values.forEach(v => paramsList.push(encodeURIComponent(k) + '=' + encodeURIComponent(v))); + values.forEach( + v => paramsList.push( + this.queryEncoder.encodeKey(k) + '=' + this.queryEncoder.encodeValue(v))); }); return paramsList.join('&'); } diff --git a/modules/@angular/http/test/url_search_params_spec.ts b/modules/@angular/http/test/url_search_params_spec.ts index e6ddb6fdb1..8a96feed6a 100644 --- a/modules/@angular/http/test/url_search_params_spec.ts +++ b/modules/@angular/http/test/url_search_params_spec.ts @@ -34,12 +34,56 @@ export function main() { }); + it('should optionally accept a custom parser', () => { + let fooEveryThingParser = { + encodeKey() { return 'I AM KEY'; }, + encodeValue() { return 'I AM VALUE'; } + }; + let params = new URLSearchParams('', fooEveryThingParser); + params.set('myKey', 'myValue'); + expect(params.toString()).toBe('I AM KEY=I AM VALUE'); + }); + + it('should encode special characters in params', () => { var searchParams = new URLSearchParams(); searchParams.append('a', '1+1'); searchParams.append('b c', '2'); searchParams.append('d%', '3$'); - expect(searchParams.toString()).toEqual('a=1%2B1&b%20c=2&d%25=3%24'); + expect(searchParams.toString()).toEqual('a=1+1&b%20c=2&d%25=3$'); + }); + + + it('should not encode allowed characters', () => { + /* + * https://tools.ietf.org/html/rfc3986#section-3.4 + * Allowed: ( pchar / "/" / "?" ) + * pchar: unreserved / pct-encoded / sub-delims / ":" / "@" + * unreserved: ALPHA / DIGIT / "-" / "." / "_" / "~" + * pct-encoded: "%" HEXDIG HEXDIG + * sub-delims: "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "=" + * + * & and = are excluded and should be encoded inside keys and values + * because URLSearchParams is responsible for inserting this. + **/ + + let params = new URLSearchParams(); + '! $ \' ( ) * + , ; A 9 - . _ ~ ? /'.split(' ').forEach( + (char, idx) => { params.set(`a${idx}`, char); }); + expect(params.toString()) + .toBe( + `a0=!&a1=$&a2=\'&a3=(&a4=)&a5=*&a6=+&a7=,&a8=;&a9=A&a10=9&a11=-&a12=.&a13=_&a14=~&a15=?&a16=/` + .replace(/\s/g, '')); + + + // Original example from https://github.com/angular/angular/issues/9348 for posterity + params = new URLSearchParams(); + params.set('q', 'repo:janbaer/howcani+type:issue'); + params.set('sort', 'created'); + params.set('order', 'desc'); + params.set('page', '1'); + expect(params.toString()) + .toBe('q=repo:janbaer/howcani+type:issue&sort=created&order=desc&page=1'); }); diff --git a/tools/public_api_guard/http/index.d.ts b/tools/public_api_guard/http/index.d.ts index d9a18d5279..c8cd31291e 100644 --- a/tools/public_api_guard/http/index.d.ts +++ b/tools/public_api_guard/http/index.d.ts @@ -100,6 +100,12 @@ export declare abstract class JSONPConnection implements Connection { abstract finished(data?: any): void; } +/** @experimental */ +export declare class QueryEncoder { + encodeKey(k: string): string; + encodeValue(v: string): string; +} + /** @experimental */ export declare enum ReadyState { Unsent = 0, @@ -209,7 +215,7 @@ export declare enum ResponseType { export declare class URLSearchParams { paramsMap: Map; rawParams: string; - constructor(rawParams?: string); + constructor(rawParams?: string, queryEncoder?: QueryEncoder); append(param: string, val: string): void; appendAll(searchParams: URLSearchParams): void; clone(): URLSearchParams;