Permalink
Browse files

fix(http): headers should be case-insensitive.

  • Loading branch information...
asnowwolf authored and vicb committed Jul 26, 2016
1 parent e34a04d commit 7f647822bdddaad6a96c4f368a9874ce7ae3775c
@@ -152,18 +152,18 @@ export class XHRConnection implements Connection {
case ContentType.NONE:
break;
case ContentType.JSON:
_xhr.setRequestHeader('Content-Type', 'application/json');
_xhr.setRequestHeader('content-type', 'application/json');
break;
case ContentType.FORM:
_xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded;charset=UTF-8');
_xhr.setRequestHeader('content-type', 'application/x-www-form-urlencoded;charset=UTF-8');
break;
case ContentType.TEXT:
_xhr.setRequestHeader('Content-Type', 'text/plain');
_xhr.setRequestHeader('content-type', 'text/plain');
break;
case ContentType.BLOB:
var blob = req.blob();
if (blob.type) {
_xhr.setRequestHeader('Content-Type', blob.type);
_xhr.setRequestHeader('content-type', blob.type);
}
break;
}
@@ -56,7 +56,7 @@ export class Headers {
// headers instanceof StringMap
StringMapWrapper.forEach(headers, (v: any, k: string) => {
this._headersMap.set(k, isListLikeIterable(v) ? v : [v]);
this._headersMap.set(normalize(k), isListLikeIterable(v) ? v : [v]);

This comment has been minimized.

Show comment
Hide comment
@cyrilhamidechi

cyrilhamidechi Sep 14, 2016

Hi all,

https://tools.ietf.org/html/rfc2616#page-31
"Field names are case-insensitive."

As far as I understand, this is a server-side concern, not client-side.
Server has to be case-insensitive.
There is no statements saying that "headers sent by the client should be lowercased".
This is an amazing breaking change.

Or maybe I've missed something?

@cyrilhamidechi

cyrilhamidechi Sep 14, 2016

Hi all,

https://tools.ietf.org/html/rfc2616#page-31
"Field names are case-insensitive."

As far as I understand, this is a server-side concern, not client-side.
Server has to be case-insensitive.
There is no statements saying that "headers sent by the client should be lowercased".
This is an amazing breaking change.

Or maybe I've missed something?

This comment has been minimized.

Show comment
Hide comment
@vicb

vicb Sep 14, 2016

Contributor
setHeader('abc', v1)
setHeader('Abc', v2)

this would have sent 2 headers before.

In what is that a breaking change ? (open an issue if this is a real concern)

@vicb

vicb Sep 14, 2016

Contributor
setHeader('abc', v1)
setHeader('Abc', v2)

this would have sent 2 headers before.

In what is that a breaking change ? (open an issue if this is a real concern)

This comment has been minimized.

Show comment
Hide comment
@cyrilhamidechi

cyrilhamidechi Sep 15, 2016

Thank you for your answer @vicb .
Issue has been opened here #11624

@cyrilhamidechi

cyrilhamidechi Sep 15, 2016

Thank you for your answer @vicb .
Issue has been opened here #11624

});
}
@@ -68,13 +68,16 @@ export class Headers {
.split('\n')
.map(val => val.split(':'))
.map(([key, ...parts]) => ([key.trim(), parts.join(':').trim()]))
.reduce((headers, [key, value]) => !headers.set(key, value) && headers, new Headers());
.reduce(
(headers, [key, value]) => !headers.set(normalize(key), value) && headers,
new Headers());
}
/**
* Appends a header to existing list of header values for a given header name.
*/
append(name: string, value: string): void {
name = normalize(name);
var mapName = this._headersMap.get(name);
var list = isListLikeIterable(mapName) ? mapName : [];
list.push(value);
@@ -84,7 +87,7 @@ export class Headers {
/**
* Deletes all header values for the given name.
*/
delete (name: string): void { this._headersMap.delete(name); }
delete (name: string): void { this._headersMap.delete(normalize(name)); }
forEach(fn: (values: string[], name: string, headers: Map<string, string[]>) => void): void {
this._headersMap.forEach(fn);
@@ -93,12 +96,12 @@ export class Headers {
/**
* Returns first header that matches given name.
*/
get(header: string): string { return ListWrapper.first(this._headersMap.get(header)); }
get(header: string): string { return ListWrapper.first(this._headersMap.get(normalize(header))); }
/**
* Check for existence of header by given name.
*/
has(header: string): boolean { return this._headersMap.has(header); }
has(header: string): boolean { return this._headersMap.has(normalize(header)); }
/**
* Provides names of set headers
@@ -118,7 +121,7 @@ export class Headers {
list.push(<string>value);
}
this._headersMap.set(header, list);
this._headersMap.set(normalize(header), list);
}
/**
@@ -137,7 +140,7 @@ export class Headers {
iterateListLike(
values, (val: any /** TODO #9100 */) => list = ListWrapper.concat(list, val.split(',')));
(serializableHeaders as any /** TODO #9100 */)[name] = list;
(serializableHeaders as any /** TODO #9100 */)[normalize(name)] = list;
});
return serializableHeaders;
}
@@ -146,7 +149,7 @@ export class Headers {
* Returns list of header values for a given name.
*/
getAll(header: string): string[] {
var headers = this._headersMap.get(header);
var headers = this._headersMap.get(normalize(header));
return isListLikeIterable(headers) ? headers : [];
}
@@ -155,3 +158,11 @@ export class Headers {
*/
entries() { throw new BaseException('"entries" method is not implemented on Headers class'); }
}
// "HTTP character sets are identified by case-insensitive tokens"
// Spec at https://tools.ietf.org/html/rfc2616
// This implementation is same as NodeJS.
// see https://nodejs.org/dist/latest-v6.x/docs/api/http.html#http_message_headers
function normalize(name: string): string {
return name.toLowerCase();
}
@@ -236,9 +236,9 @@ export function main() {
var connection = new XHRConnection(
new Request(base.merge(new RequestOptions({headers: headers}))), new MockBrowserXHR());
connection.response.subscribe();
expect(setRequestHeaderSpy).toHaveBeenCalledWith('Content-Type', 'text/xml');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('Breaking-Bad', '<3');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('X-Multi', 'a,b');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('content-type', 'text/xml');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('breaking-bad', '<3');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('x-multi', 'a,b');
});
it('should skip content type detection if custom content type header is set', () => {
@@ -249,8 +249,8 @@ export function main() {
new Request(base.merge(new RequestOptions({body: body, headers: headers}))),
new MockBrowserXHR());
connection.response.subscribe();
expect(setRequestHeaderSpy).toHaveBeenCalledWith('Content-Type', 'text/plain');
expect(setRequestHeaderSpy).not.toHaveBeenCalledWith('Content-Type', 'application/json');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('content-type', 'text/plain');
expect(setRequestHeaderSpy).not.toHaveBeenCalledWith('content-type', 'application/json');
});
it('should use object body and detect content type header to the request', () => {
@@ -260,7 +260,7 @@ export function main() {
new Request(base.merge(new RequestOptions({body: body}))), new MockBrowserXHR());
connection.response.subscribe();
expect(sendSpy).toHaveBeenCalledWith(Json.stringify(body));
expect(setRequestHeaderSpy).toHaveBeenCalledWith('Content-Type', 'application/json');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('content-type', 'application/json');
});
it('should use number body and detect content type header to the request', () => {
@@ -270,7 +270,7 @@ export function main() {
new Request(base.merge(new RequestOptions({body: body}))), new MockBrowserXHR());
connection.response.subscribe();
expect(sendSpy).toHaveBeenCalledWith('23');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('Content-Type', 'text/plain');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('content-type', 'text/plain');
});
it('should use string body and detect content type header to the request', () => {
@@ -280,7 +280,7 @@ export function main() {
new Request(base.merge(new RequestOptions({body: body}))), new MockBrowserXHR());
connection.response.subscribe();
expect(sendSpy).toHaveBeenCalledWith(body);
expect(setRequestHeaderSpy).toHaveBeenCalledWith('Content-Type', 'text/plain');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('content-type', 'text/plain');
});
it('should use URLSearchParams body and detect content type header to the request', () => {
@@ -294,7 +294,7 @@ export function main() {
expect(sendSpy).toHaveBeenCalledWith('test1=val1&test2=val2');
expect(setRequestHeaderSpy)
.toHaveBeenCalledWith(
'Content-Type', 'application/x-www-form-urlencoded;charset=UTF-8');
'content-type', 'application/x-www-form-urlencoded;charset=UTF-8');
});
if ((global as any /** TODO #9100 */)['Blob']) {
@@ -335,7 +335,7 @@ export function main() {
new Request(base.merge(new RequestOptions({body: body}))), new MockBrowserXHR());
connection.response.subscribe();
expect(sendSpy).toHaveBeenCalledWith(body);
expect(setRequestHeaderSpy).toHaveBeenCalledWith('Content-Type', 'text/css');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('content-type', 'text/css');
});
it('should use blob body without type to the request', () => {
@@ -358,7 +358,7 @@ export function main() {
new MockBrowserXHR());
connection.response.subscribe();
expect(sendSpy).toHaveBeenCalledWith(body);
expect(setRequestHeaderSpy).toHaveBeenCalledWith('Content-Type', 'text/css');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('content-type', 'text/css');
});
it('should use array buffer body to the request', () => {
@@ -389,7 +389,7 @@ export function main() {
new MockBrowserXHR());
connection.response.subscribe();
expect(sendSpy).toHaveBeenCalledWith(body);
expect(setRequestHeaderSpy).toHaveBeenCalledWith('Content-Type', 'text/css');
expect(setRequestHeaderSpy).toHaveBeenCalledWith('content-type', 'text/css');
});
}
@@ -20,6 +20,10 @@ export function main() {
var firstHeaders = new Headers(); // Currently empty
firstHeaders.append('Content-Type', 'image/jpeg');
expect(firstHeaders.get('Content-Type')).toBe('image/jpeg');
// "HTTP character sets are identified by case-insensitive tokens"
// Spec at https://tools.ietf.org/html/rfc2616
expect(firstHeaders.get('content-type')).toBe('image/jpeg');
expect(firstHeaders.get('content-Type')).toBe('image/jpeg');
var httpHeaders = StringMapWrapper.create();
StringMapWrapper.set(httpHeaders, 'Content-Type', 'image/jpeg');
StringMapWrapper.set(httpHeaders, 'Accept-Charset', 'utf-8');
@@ -72,7 +76,7 @@ export function main() {
beforeEach(() => {
headers = new Headers();
inputArr = ['application/jeisen', 'application/jason', 'application/patrickjs'];
obj = {'Accept': inputArr};
obj = {'accept': inputArr};
headers.set('Accept', inputArr);
});

1 comment on commit 7f64782

@gfrSE

This comment has been minimized.

Show comment
Hide comment
@gfrSE

gfrSE Aug 11, 2016

What about this line?

gfrSE commented on 7f64782 Aug 11, 2016

What about this line?

Please sign in to comment.