From 5215f642c25a3d8bac6e4b26c876177b9ba7eadd Mon Sep 17 00:00:00 2001 From: Ben Lesh <ben@benlesh.com> Date: Wed, 2 Sep 2020 15:09:58 -0500 Subject: [PATCH] fix(ajax): Allow XHR to perform body serialization and set content-type where possible - Ajax will now allow default behavior as defined in the specs: - https://xhr.spec.whatwg.org/#the-send()-method - https://fetch.spec.whatwg.org/#concept-bodyinit-extract - if the `body` is set to: `Blob`, `ArrayBuffer`, any array buffer view (like a byte sequence, e.g. `Uint8Array`, etc), `FormData`, `URLSearchParams`, `string`, or `ReadableStream`, default handling is use. If the `body` is otherwise `typeof` `"object"`, then it will be converted to JSON via `JSON.stringify`, and the `Content-Type` header will be set to `application/json;charset=utf-8`. All other types will emit an error. - Updates tests. - Puts the behavior inline with axios, et al. resolves #2837 --- package-lock.json | 47 +++++++++++++ package.json | 1 + spec/observables/dom/ajax-spec.ts | 105 ++++++----------------------- src/internal/ajax/ajax.ts | 108 +++++++++++++++++++++++------- src/internal/ajax/types.ts | 4 ++ 5 files changed, 155 insertions(+), 110 deletions(-) diff --git a/package-lock.json b/package-lock.json index 3cd84e87a5..19e95f8282 100644 --- a/package-lock.json +++ b/package-lock.json @@ -714,6 +714,12 @@ "integrity": "sha512-z/WhQ5FPySLdvREByI2vZiTWwCnF0moMJ1hK9YQwDTHKh6I7/uSckMetoRGb5UBZPC1z0jlw+n/XCgjeH7y1AQ==", "dev": true }, + "asynckit": { + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", + "integrity": "sha1-x57Zf380y48robyXkLzDZkdLS3k=", + "dev": true + }, "atob": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/atob/-/atob-2.1.2.tgz", @@ -1622,6 +1628,15 @@ "integrity": "sha1-FopHAXVran9RoSzgyXv6KMCE7WM=", "dev": true }, + "combined-stream": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/combined-stream/-/combined-stream-1.0.8.tgz", + "integrity": "sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==", + "dev": true, + "requires": { + "delayed-stream": "~1.0.0" + } + }, "commander": { "version": "2.20.0", "resolved": "https://registry.npmjs.org/commander/-/commander-2.20.0.tgz", @@ -1981,6 +1996,12 @@ } } }, + "delayed-stream": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/delayed-stream/-/delayed-stream-1.0.0.tgz", + "integrity": "sha1-3zrhmayt+31ECqrgsp4icrJOxhk=", + "dev": true + }, "dependency-cruiser": { "version": "4.27.3", "resolved": "https://registry.npmjs.org/dependency-cruiser/-/dependency-cruiser-4.27.3.tgz", @@ -3000,6 +3021,17 @@ "integrity": "sha1-gQaNKVqBQuwKxybG4iAMMPttXoA=", "dev": true }, + "form-data": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/form-data/-/form-data-3.0.0.tgz", + "integrity": "sha512-CKMFDglpbMi6PyN+brwB9Q/GOw0eAnsrEZDgcsH5Krhz5Od/haKHAX0NmQfha2zPPz0JpWzA7GJHGSnvCRLWsg==", + "dev": true, + "requires": { + "asynckit": "^0.4.0", + "combined-stream": "^1.0.8", + "mime-types": "^2.1.12" + } + }, "fragment-cache": { "version": "0.2.1", "resolved": "https://registry.npmjs.org/fragment-cache/-/fragment-cache-0.2.1.tgz", @@ -5201,6 +5233,21 @@ "brorand": "^1.0.1" } }, + "mime-db": { + "version": "1.44.0", + "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.44.0.tgz", + "integrity": "sha512-/NOTfLrsPBVeH7YtFPgsVWveuL+4SjjYxaQ1xtM1KMFj7HdxlBlxeyNLzhyJVx7r4rZGJAZ/6lkKCitSc/Nmpg==", + "dev": true + }, + "mime-types": { + "version": "2.1.27", + "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-2.1.27.tgz", + "integrity": "sha512-JIhqnCasI9yD+SsmkquHBxTSEuZdQX5BuQnS2Vc7puQQQ+8yiP5AY5uWhpdv4YL4VM5c6iliiYWPgJ/nJQLp7w==", + "dev": true, + "requires": { + "mime-db": "1.44.0" + } + }, "mimic-fn": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/mimic-fn/-/mimic-fn-1.2.0.tgz", diff --git a/package.json b/package.json index ec43086b75..2c523446de 100644 --- a/package.json +++ b/package.json @@ -117,6 +117,7 @@ "escape-string-regexp": "1.0.5", "eslint": "4.18.2", "eslint-plugin-jasmine": "^2.10.1", + "form-data": "^3.0.0", "fs-extra": "^8.1.0", "glob": "7.1.2", "google-closure-compiler-js": "20170218.0.0", diff --git a/spec/observables/dom/ajax-spec.ts b/spec/observables/dom/ajax-spec.ts index 4a10cb7066..a8e9a8414f 100644 --- a/spec/observables/dom/ajax-spec.ts +++ b/spec/observables/dom/ajax-spec.ts @@ -4,9 +4,16 @@ import * as sinon from 'sinon'; import { ajax, AjaxConfig, AjaxResponse, AjaxError, AjaxTimeoutError } from 'rxjs/ajax'; import { TestScheduler } from 'rxjs/testing'; import { noop } from 'rxjs'; +import * as nodeFormData from 'form-data'; + + const root: any = (typeof globalThis !== 'undefined' && globalThis) || (typeof self !== 'undefined' && self) || global; +if (typeof root.FormData === 'undefined') { + root.FormData = nodeFormData as any; +} + /** @test {ajax} */ describe('ajax', () => { let rXHR: XMLHttpRequest; @@ -495,21 +502,10 @@ describe('ajax', () => { }); describe('ajax request body', () => { - let rFormData: FormData; - - beforeEach(() => { - rFormData = root.FormData; - root.FormData = root.FormData || class {}; - }); - - afterEach(() => { - root.FormData = rFormData; - }); - it('can take string body', () => { const obj = { url: '/flibbertyJibbet', - method: '', + method: 'POST', body: 'foobar', }; @@ -523,53 +519,33 @@ describe('ajax', () => { const body = new root.FormData(); const obj = { url: '/flibbertyJibbet', - method: '', + method: 'POST', body: body, }; ajax(obj).subscribe(); expect(MockXMLHttpRequest.mostRecent.url).to.equal('/flibbertyJibbet'); - expect(MockXMLHttpRequest.mostRecent.data).to.deep.equal(body); + expect(MockXMLHttpRequest.mostRecent.data).to.equal(body); expect(MockXMLHttpRequest.mostRecent.requestHeaders).to.deep.equal({ 'x-requested-with': 'XMLHttpRequest', }); }); - it('should not fail when FormData is undefined', () => { - root.FormData = void 0; - - const obj = { - url: '/flibbertyJibbet', - method: '', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, - body: { '🌟': '🚀' }, - }; - - ajax(obj).subscribe(); - - expect(MockXMLHttpRequest.mostRecent.url).to.equal('/flibbertyJibbet'); - }); - - it('should send by form-urlencoded format', () => { - const body = { + it('should send the URLSearchParams straight through to the body', () => { + const body = new URLSearchParams({ '🌟': '🚀', - }; + }); const obj = { url: '/flibbertyJibbet', - method: '', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, + method: 'POST', body: body, }; ajax(obj).subscribe(); expect(MockXMLHttpRequest.mostRecent.url).to.equal('/flibbertyJibbet'); - expect(MockXMLHttpRequest.mostRecent.data).to.equal('%F0%9F%8C%9F=%F0%9F%9A%80'); + expect(MockXMLHttpRequest.mostRecent.data).to.equal(body); }); it('should send by JSON', () => { @@ -578,10 +554,7 @@ describe('ajax', () => { }; const obj = { url: '/flibbertyJibbet', - method: '', - headers: { - 'Content-Type': 'application/json', - }, + method: 'POST', body: body, }; @@ -601,7 +574,7 @@ describe('ajax', () => { method: '', body: body, headers: { - 'cOnTeNt-TyPe': 'application/json; charset=UTF-8', + 'cOnTeNt-TyPe': 'application/json;charset=UTF-8', }, }; @@ -753,42 +726,7 @@ describe('ajax', () => { expect(request.method).to.equal('POST'); expect(request.url).to.equal('/flibbertyJibbet'); expect(request.requestHeaders).to.deep.equal({ - 'content-type': 'application/x-www-form-urlencoded; charset=UTF-8', - 'x-requested-with': 'XMLHttpRequest', - }); - - request.respondWith({ - status: 200, - contentType: 'application/json', - responseText: JSON.stringify(expected), - }); - - expect(request.data).to.equal('foo=bar&hi=there%20you'); - expect(result!.response).to.deep.equal(expected); - expect(complete).to.be.true; - }); - - it('should properly encode full URLs passed', () => { - const expected = { test: 'https://google.com/search?q=encodeURI+vs+encodeURIComponent' }; - let result: AjaxResponse<any>; - let complete = false; - - ajax.post('/flibbertyJibbet', expected).subscribe( - (x) => { - result = x; - }, - null, - () => { - complete = true; - } - ); - - const request = MockXMLHttpRequest.mostRecent; - - expect(request.method).to.equal('POST'); - expect(request.url).to.equal('/flibbertyJibbet'); - expect(request.requestHeaders).to.deep.equal({ - 'content-type': 'application/x-www-form-urlencoded; charset=UTF-8', + 'content-type': 'application/json;charset=utf-8', 'x-requested-with': 'XMLHttpRequest', }); @@ -798,7 +736,7 @@ describe('ajax', () => { responseText: JSON.stringify(expected), }); - expect(request.data).to.equal('test=https%3A%2F%2Fgoogle.com%2Fsearch%3Fq%3DencodeURI%2Bvs%2BencodeURIComponent'); + expect(request.data).to.equal(JSON.stringify(expected)); expect(result!.response).to.deep.equal(expected); expect(complete).to.be.true; }); @@ -823,7 +761,6 @@ describe('ajax', () => { expect(request.method).to.equal('POST'); expect(request.url).to.equal('/flibbertyJibbet'); expect(request.requestHeaders).to.deep.equal({ - 'content-type': 'application/x-www-form-urlencoded; charset=UTF-8', 'x-requested-with': 'XMLHttpRequest', }); @@ -1043,7 +980,7 @@ describe('ajax', () => { expect(request.method).to.equal('PATCH'); expect(request.url).to.equal('/flibbertyJibbet'); expect(request.requestHeaders).to.deep.equal({ - 'content-type': 'application/x-www-form-urlencoded; charset=UTF-8', + 'content-type': 'application/json;charset=utf-8', 'x-requested-with': 'XMLHttpRequest', }); @@ -1053,7 +990,7 @@ describe('ajax', () => { responseText: JSON.stringify(expected), }); - expect(request.data).to.equal('foo=bar&hi=there%20you'); + expect(request.data).to.equal(JSON.stringify(expected)); expect(result!.response).to.deep.equal(expected); expect(complete).to.be.true; }); diff --git a/src/internal/ajax/ajax.ts b/src/internal/ajax/ajax.ts index 4924d06ec4..e435d2faba 100644 --- a/src/internal/ajax/ajax.ts +++ b/src/internal/ajax/ajax.ts @@ -269,10 +269,6 @@ export const ajax: AjaxCreationMethod = (() => { return create; })(); -function isFormData(body: any): body is FormData { - return typeof FormData !== 'undefined' && body instanceof FormData; -} - export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> { return new Observable<AjaxResponse<T>>((destination) => { let done = false; @@ -301,13 +297,17 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> { headers['x-requested-with'] = 'XMLHttpRequest'; } - // Ensure content type is set - if (!('content-type' in headers) && config.body !== undefined && !isFormData(config.body)) { - headers['content-type'] = 'application/x-www-form-urlencoded; charset=UTF-8'; + // Examine the body and determine whether or not to serialize it + // and set the content-type in `headers`, if we're able. + let body: any; + try { + body = extractContentTypeAndMaybeSerializeBody(config.body, headers); + } catch (err) { + // We might land here because of JSON.stringify issues, like circular refrences. + destination.error(err); + return; } - const body: string | undefined = serializeBody(config.body, headers['content-type']); - const _request: AjaxRequest = { // Default values async: true, @@ -346,7 +346,7 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> { const progressSubscriber = config.progressSubscriber; - xhr.ontimeout = (e: ProgressEvent) => { + xhr.ontimeout = () => { const timeoutError = new AjaxTimeoutError(xhr, _request); progressSubscriber?.error?.(timeoutError); destination.error(timeoutError); @@ -431,26 +431,82 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> { }); } -function serializeBody(body: any, contentType?: string) { - if (!body || typeof body === 'string' || isFormData(body)) { +/** + * Examines the body to determine if we need to serialize it for them or not. + * If the body is a type that XHR handles natively, we just allow it through, + * otherwise, if the body is something that *we* can serialize for the user, + * we will serialize it, and attempt to set the `content-type` header, if it's + * not already set. + * @param body The body passed in by the user + * @param headers The normalized headers + */ +function extractContentTypeAndMaybeSerializeBody(body: any, headers: Record<string, string>) { + if ( + !body || + typeof body === 'string' || + isFormData(body) || + isURLSearchParams(body) || + isArrayBuffer(body) || + isFile(body) || + isBlob(body) || + isReadableStream(body) + ) { + // The XHR instance itself can handle serializing these, and set the content-type for us + // so we don't need to do that. https://xhr.spec.whatwg.org/#the-send()-method return body; } - if (contentType) { - const splitIndex = contentType.indexOf(';'); - if (splitIndex !== -1) { - contentType = contentType.substring(0, splitIndex); - } + if (isArrayBufferView(body)) { + // This is a typed array (e.g. Float32Array or Uint8Array), or a DataView. + // XHR can handle this one too: https://fetch.spec.whatwg.org/#concept-bodyinit-extract + return body.buffer; } - switch (contentType) { - case 'application/x-www-form-urlencoded': - return Object.keys(body) - .map((key) => `${encodeURIComponent(key)}=${encodeURIComponent(body[key])}`) - .join('&'); - case 'application/json': - return JSON.stringify(body); - default: - return body; + if (typeof body === 'object') { + // If we have made it here, this is an object, probably a POJO, and we'll try + // to serialize it for them. If this doesn't work, it will throw, obviously, which + // is okay. The workaround for users would be to manually set the body to their own + // serialized string (accounting for circular refrences or whatever), then set + // the content-type manually as well. + headers['content-type'] = headers['content-type'] ?? 'application/json;charset=utf-8'; + return JSON.stringify(body); } + + // If we've gotten past everything above, this is something we don't quite know how to + // handle. Throw an error. This will be caught and emitted from the observable. + throw new TypeError('Unknown body type'); +} + +const _toString = Object.prototype.toString; + +function toStringCheck(obj: any, name: string): boolean { + return _toString.call(obj) === `[object ${name}]`; +} + +function isArrayBuffer(body: any): body is ArrayBuffer { + return toStringCheck(body, 'ArrayBuffer'); +} + +function isFile(body: any): body is File { + return toStringCheck(body, 'File'); +} + +function isBlob(body: any): body is Blob { + return toStringCheck(body, 'Blob'); +} + +function isArrayBufferView(body: any): body is ArrayBufferView { + return typeof ArrayBuffer !== 'undefined' && ArrayBuffer.isView(body); +} + +function isFormData(body: any): body is FormData { + return typeof FormData !== 'undefined' && body instanceof FormData; +} + +function isURLSearchParams(body: any): body is URLSearchParams { + return typeof URLSearchParams !== 'undefined' && body instanceof URLSearchParams; +} + +function isReadableStream(body: any): body is ReadableStream { + return typeof ReadableStream !== 'undefined' && body instanceof ReadableStream; } diff --git a/src/internal/ajax/types.ts b/src/internal/ajax/types.ts index d1c58f2946..bbdbba1c5b 100644 --- a/src/internal/ajax/types.ts +++ b/src/internal/ajax/types.ts @@ -149,6 +149,10 @@ export interface AjaxConfig { * wish to override the default `XMLHttpRequest` for some reason. * * If not provided, the `XMLHttpRequest` in global scope will be used. + * + * NOTE: This AJAX implementation relies on the built-in serialization and setting + * of Content-Type headers that is provided by standards-compliant XMLHttpRequest implementations, + * be sure any implementation you use meets that standard. */ createXHR?: () => XMLHttpRequest;