Skip to content

Commit

Permalink
Fix problems with merging headers with undefined Content-Type
Browse files Browse the repository at this point in the history
  • Loading branch information
eliperelman committed Apr 30, 2019
1 parent e0c5c1b commit 6fe3a69
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 24 deletions.
6 changes: 4 additions & 2 deletions src/core/public/http/http_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,16 +200,18 @@ describe('http requests', async () => {
it('should make requests for NDJSON content', async () => {
const { http } = setupService();
const content = readFileSync(join(__dirname, '_import_objects.ndjson'), { encoding: 'utf-8' });
const body = new FormData();

body.append('file', content);
fetchMock.post('*', {
body: content,
headers: { 'Content-Type': 'application/ndjson' },
});

const data = await http.post('/my/path', {
body: content,
body,
headers: {
'Content-Type': 'multipart/form-data',
'Content-Type': undefined,
},
});

Expand Down
30 changes: 18 additions & 12 deletions src/core/public/http/http_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,34 @@ export class HttpService {
private readonly stop$ = new Rx.Subject();

public setup({ basePath, injectedMetadata, fatalErrors }: Deps) {
const defaults: HttpFetchOptions = {
method: 'GET',
credentials: 'same-origin',
prependBasePath: true,
headers: {
'Content-Type': 'application/json',
'kbn-version': injectedMetadata.getKibanaVersion(),
},
};

async function fetch(path: string, options: HttpFetchOptions = {}): Promise<HttpBody> {
const { query, prependBasePath, ...fetchOptions } = merge({}, defaults, options);
const { query, prependBasePath, ...fetchOptions } = merge({
method: 'GET',
credentials: 'same-origin',
prependBasePath: true,
headers: {
'kbn-version': injectedMetadata.getKibanaVersion(),
'Content-Type': 'application/json',
}
}, options);
const url = format({
pathname: prependBasePath ? basePath.addToPath(path) : path,
query,
});

if (
options.headers &&
'Content-Type' in options.headers &&
options.headers['Content-Type'] === undefined
) {
delete options.headers['Content-Type'];
}

let response;
let body = null;

try {
response = await window.fetch(url, fetchOptions);
response = await window.fetch(url, fetchOptions as RequestInit);
} catch (err) {
throw new HttpFetchError(err.message);
}
Expand Down
21 changes: 20 additions & 1 deletion src/core/public/http/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,24 @@ import { BasePathSetup } from '../base_path';
import { InjectedMetadataSetup } from '../injected_metadata';
import { FatalErrorsSetup } from '../fatal_errors';

export interface HttpHeadersInit {
[name: string]: any;
}
export interface HttpRequestInit {
body?: BodyInit | null;
cache?: RequestCache;
credentials?: RequestCredentials;
headers?: HttpHeadersInit;
integrity?: string;
keepalive?: boolean;
method?: string;
mode?: RequestMode;
redirect?: RequestRedirect;
referrer?: string;
referrerPolicy?: ReferrerPolicy;
signal?: AbortSignal | null;
window?: any;
}
export interface Deps {
basePath: BasePathSetup;
injectedMetadata: InjectedMetadataSetup;
Expand All @@ -29,9 +47,10 @@ export interface Deps {
export interface HttpFetchQuery {
[key: string]: string | number | boolean | undefined;
}
export interface HttpFetchOptions extends RequestInit {
export interface HttpFetchOptions extends HttpRequestInit {
query?: HttpFetchQuery;
prependBasePath?: boolean;
headers?: HttpHeadersInit;
}
export interface Abortable<T> {
abort: () => Promise<T>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ export async function importFile(file, overwriteAll = false) {
pathname: '/api/saved_objects/_import',
body: formData,
headers: {
'Content-Type': 'multipart/form-data',
// Important to be undefined, it forces proper headers to be set for FormData
'Content-Type': undefined,
},
query: {
overwrite: overwriteAll
Expand Down
28 changes: 20 additions & 8 deletions src/legacy/ui/public/kfetch/kfetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ import { merge } from 'lodash';
import { KFetchError } from './kfetch_error';

import { HttpSetup } from '../../../../core/public';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { HttpRequestInit } from '../../../../core/public/http/types';

export interface KFetchQuery {
[key: string]: string | number | boolean | undefined;
}

export interface KFetchOptions extends RequestInit {
export interface KFetchOptions extends HttpRequestInit {
pathname: string;
query?: KFetchQuery;
}
Expand Down Expand Up @@ -100,18 +102,28 @@ function responseInterceptors(responsePromise: Promise<any>) {
}

export function withDefaultOptions(options?: KFetchOptions): KFetchOptions {
return merge(
const withDefaults = merge(
{
method: 'GET',
credentials: 'same-origin',
headers: {
...(options && options.headers && options.headers.hasOwnProperty('Content-Type')
? {}
: {
'Content-Type': 'application/json',
}),
'Content-Type': 'application/json',
},
},
options
);
) as KFetchOptions;

if (
options &&
options.headers &&
'Content-Type' in options.headers &&
options.headers['Content-Type'] === undefined
) {
// TS thinks headers could be undefined here, but that isn't possible because
// of the merge above.
// @ts-ignore
withDefaults.headers['Content-Type'] = undefined;
}

return withDefaults;
}

0 comments on commit 6fe3a69

Please sign in to comment.