Skip to content

Commit

Permalink
fix(http): correctly handle response body for 204 status code
Browse files Browse the repository at this point in the history
  • Loading branch information
Dzmitry Shylovich authored and Dzmitry Shylovich committed Nov 12, 2016
1 parent 13554e6 commit b49603c
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 18 deletions.
40 changes: 26 additions & 14 deletions modules/@angular/http/src/backends/xhr_backend.ts
Expand Up @@ -7,16 +7,18 @@
*/
import {Injectable} from '@angular/core';
import {__platform_browser_private__} from '@angular/platform-browser';
import {Observable} from 'rxjs/Observable';
import {Observer} from 'rxjs/Observer';

import {ResponseOptions} from '../base_response_options';
import {ContentType, ReadyState, RequestMethod, ResponseContentType, ResponseType} from '../enums';
import {Headers} from '../headers';
import {getResponseURL, isSuccess} from '../http_utils';
import {Connection, ConnectionBackend, XSRFStrategy} from '../interfaces';
import {Request} from '../static_request';
import {Response} from '../static_response';

import {BrowserXhr} from './browser_xhr';
import {Observable} from 'rxjs/Observable';
import {Observer} from 'rxjs/Observer';

const XSSI_PREFIX = /^\)\]\}',?\n/;

Expand Down Expand Up @@ -48,27 +50,37 @@ export class XHRConnection implements Connection {
}
// load event handler
const onLoad = () => {
// responseText is the old-school way of retrieving response (supported by IE8 & 9)
// response/responseType properties were introduced in ResourceLoader Level2 spec (supported
// by IE10)
let body = _xhr.response === undefined ? _xhr.responseText : _xhr.response;
// Implicitly strip a potential XSSI prefix.
if (typeof body === 'string') body = body.replace(XSSI_PREFIX, '');
const headers = Headers.fromResponseHeaderString(_xhr.getAllResponseHeaders());

const url = getResponseURL(_xhr);

// normalize IE9 bug (http://bugs.jquery.com/ticket/1450)
let status: number = _xhr.status === 1223 ? 204 : _xhr.status;

let body: any = null;

// 204 is not supposed to have any response body
if (status !== 204) {
// responseText is the old-school way of retrieving response (supported by IE8 & 9)
// response/responseType properties were introduced in ResourceLoader Level2 spec
// (supported
// by IE10)
body = _xhr.response == null ? _xhr.responseText : _xhr.response;

// Implicitly strip a potential XSSI prefix.
if (typeof body === 'string') {
body = body.replace(XSSI_PREFIX, '');
}
}

// fix status code when it is 0 (0 status is undocumented).
// Occurs when accessing file resources or on Android 4.1 stock browser
// while retrieving files from application cache.
if (status === 0) {
status = body ? 200 : 0;
}

const statusText = _xhr.statusText || 'OK';
const headers: Headers = Headers.fromResponseHeaderString(_xhr.getAllResponseHeaders());

const url: string = getResponseURL(_xhr);

const statusText: string = _xhr.statusText || 'OK';

let responseOptions = new ResponseOptions({body, status, headers, statusText, url});
if (baseResponseOptions != null) {
Expand Down Expand Up @@ -137,7 +149,7 @@ export class XHRConnection implements Connection {
});
}

setDetectedContentType(req: any /** TODO #9100 */, _xhr: XMLHttpRequest) {
setDetectedContentType(req: Request, _xhr: XMLHttpRequest) {
// Skip if a custom Content-Type header is provided
if (req.headers != null && req.headers.get('Content-Type') != null) {
return;
Expand Down
7 changes: 5 additions & 2 deletions modules/@angular/http/src/static_request.ts
Expand Up @@ -67,8 +67,11 @@ export class Request extends Body {
headers: Headers;
/** Url of the remote resource */
url: string;
/** Type of the request body **/
private contentType: ContentType;
/**
* Type of the request body
* @internal
*/
contentType: ContentType;
/** Enable use credentials */
withCredentials: boolean;
/** Buffer to store the response */
Expand Down
17 changes: 16 additions & 1 deletion modules/@angular/http/test/backends/xhr_backend_spec.ts
Expand Up @@ -9,7 +9,6 @@
import {Injectable} from '@angular/core';
import {AsyncTestCompleter, SpyObject, afterEach, beforeEach, beforeEachProviders, describe, expect, inject, it} from '@angular/core/testing/testing_internal';
import {__platform_browser_private__} from '@angular/platform-browser';

import {BrowserXhr} from '../../src/backends/browser_xhr';
import {CookieXSRFStrategy, XHRBackend, XHRConnection} from '../../src/backends/xhr_backend';
import {BaseRequestOptions, RequestOptions} from '../../src/base_request_options';
Expand Down Expand Up @@ -486,6 +485,7 @@ export function main() {
existingXHRs[0].setStatusCode(statusCode);
existingXHRs[0].dispatchEvent('load');
}));

it('should normalize IE\'s 1223 status code into 204',
inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
var statusCode = 1223;
Expand All @@ -502,6 +502,21 @@ export function main() {
existingXHRs[0].dispatchEvent('load');
}));

it('should not throw for 204 statusCode',
inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
const statusCode = 204;
const connection = new XHRConnection(
sampleRequest, new MockBrowserXHR(), new ResponseOptions({status: statusCode}));

connection.response.subscribe((res: Response) => {
expect(res.text()).toBe('');
async.done();
});

existingXHRs[0].setStatusCode(statusCode);
existingXHRs[0].dispatchEvent('load');
}));

it('should normalize responseText and response',
inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
var responseBody = 'Doge';
Expand Down
2 changes: 1 addition & 1 deletion tools/public_api_guard/http/index.d.ts
Expand Up @@ -239,7 +239,7 @@ export declare class XHRConnection implements Connection {
request: Request;
response: Observable<Response>;
constructor(req: Request, browserXHR: BrowserXhr, baseResponseOptions?: ResponseOptions);
setDetectedContentType(req: any, _xhr: any): void;
setDetectedContentType(req: Request, _xhr: XMLHttpRequest): void;
}

/** @experimental */
Expand Down

0 comments on commit b49603c

Please sign in to comment.