Skip to content

Commit

Permalink
Do not set content-type on body-less requests (#1826)
Browse files Browse the repository at this point in the history
* Do not set content-type on body-less requests

* Add docs

* Remove commented code

* Implement CR suggestions

* Add change-set

* Fix change-set

* Post-rebase fix
  • Loading branch information
goce-cz authored Aug 30, 2024
1 parent f7bb00f commit b893c44
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilled-forks-rush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"openapi-fetch": minor
---

Do not set content-type on body-less requests
11 changes: 11 additions & 0 deletions docs/openapi-fetch/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,17 @@ const { data, error } = await client.PUT("/submit", {
});
```

::: tip

For convenience, `openapi-fetch` sets `Content-Type` to `application/json` automatically
for any request that provides value for the `body` parameter. When the `bodySerializer` returns an instance of `FormData`,
`Content-Type` is omitted, allowing the browser to set it automatically with the correct message part boundary.

You can also set `Content-Type` manually through `headers` object either in the fetch options,
or when instantiating the client.

:::

## Path serialization

openapi-fetch supports path serialization as [outlined in the 3.1 spec](https://swagger.io/docs/specification/serialization/#path). This happens automatically, based on the specific format in your OpenAPI schema:
Expand Down
28 changes: 15 additions & 13 deletions packages/openapi-fetch/src/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
// settings & const
const DEFAULT_HEADERS = {
"Content-Type": "application/json",
};

const PATH_PARAM_RE = /\{[^{}]+\}/g;

/** Add custom parameters to Request object */
Expand Down Expand Up @@ -41,7 +37,6 @@ export default function createClient(clientOptions) {
...baseOptions
} = { ...clientOptions };
baseUrl = removeTrailingSlash(baseUrl);
baseHeaders = mergeHeaders(DEFAULT_HEADERS, baseHeaders);
const middlewares = [];

/**
Expand All @@ -58,6 +53,7 @@ export default function createClient(clientOptions) {
parseAs = "json",
querySerializer: requestQuerySerializer,
bodySerializer = globalBodySerializer ?? defaultBodySerializer,
body,
...init
} = fetchOptions || {};
if (localBaseUrl) {
Expand All @@ -78,19 +74,25 @@ export default function createClient(clientOptions) {
});
}

const serializedBody = body === undefined ? undefined : bodySerializer(body);

const defaultHeaders =
// with no body, we should not to set Content-Type
serializedBody === undefined ||
// if serialized body is FormData; browser will correctly set Content-Type & boundary expression
serializedBody instanceof FormData
? {}
: {
"Content-Type": "application/json",
};

const requestInit = {
redirect: "follow",
...baseOptions,
...init,
headers: mergeHeaders(baseHeaders, headers, params.header),
body: serializedBody,
headers: mergeHeaders(defaultHeaders, baseHeaders, headers, params.header),
};
if (requestInit.body !== undefined) {
requestInit.body = bodySerializer(requestInit.body);
// remove `Content-Type` if serialized body is FormData; browser will correctly set Content-Type & boundary expression
if (requestInit.body instanceof FormData) {
requestInit.headers.delete("Content-Type");
}
}

let id;
let options;
Expand Down
144 changes: 136 additions & 8 deletions packages/openapi-fetch/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import createClient, {
type BodySerializer,
type FetchOptions,
type MethodResponse,
type HeadersOptions,
type Middleware,
type MiddlewareCallbackParams,
type QuerySerializerOptions,
type Client,
type PathBasedClient,
createPathBasedClient,
} from "../src/index.js";
import { server, baseUrl, useMockRequestHandler, toAbsoluteURL } from "./fixtures/mock-server.js";
import { baseUrl, server, toAbsoluteURL, useMockRequestHandler } from "./fixtures/mock-server.js";
import type { paths } from "./fixtures/api.js";

beforeAll(() => {
Expand Down Expand Up @@ -819,12 +820,7 @@ describe("client", () => {
await client.GET("/self");

// assert default headers were passed
expect(getRequest().headers).toEqual(
new Headers({
...headers, // assert new header got passed
"Content-Type": "application/json", // probably doesn’t need to get tested, but this was simpler than writing lots of code to ignore these
}),
);
expect(getRequest().headers).toEqual(new Headers(headers));
});

it("can be overridden", async () => {
Expand All @@ -850,7 +846,6 @@ describe("client", () => {
expect(getRequest().headers).toEqual(
new Headers({
"Cache-Control": "no-cache",
"Content-Type": "application/json",
}),
);
});
Expand Down Expand Up @@ -894,6 +889,139 @@ describe("client", () => {
});
});

describe("content-type", () => {
const BODY_ACCEPTING_METHODS = [["PUT"], ["POST"], ["DELETE"], ["OPTIONS"], ["PATCH"]] as const;
const ALL_METHODS = [...BODY_ACCEPTING_METHODS, ["GET"], ["HEAD"]] as const;

const fireRequestAndGetContentType = async (options: {
defaultHeaders?: HeadersOptions;
method: (typeof ALL_METHODS)[number][number];
fetchOptions: FetchOptions<any>;
}) => {
const client = createClient<any>({ baseUrl, headers: options.defaultHeaders });
const { getRequest } = useMockRequestHandler({
baseUrl,
method: "all",
path: "/blogposts-optional",
status: 200,
});
await client[options.method]("/blogposts-optional", options.fetchOptions as any);

const request = getRequest();
return request.headers.get("content-type");
};

it.each(ALL_METHODS)("no content-type for body-less requests - %s", async (method) => {
const contentType = await fireRequestAndGetContentType({
method,
fetchOptions: {},
});

expect(contentType).toBe(null);
});

it.each(ALL_METHODS)("no content-type for `undefined` body requests - %s", async (method) => {
const contentType = await fireRequestAndGetContentType({
method,
fetchOptions: {
body: undefined,
},
});

expect(contentType).toBe(null);
});

const BODIES = [{ prop: "a" }, {}, "", "str", null, false, 0, 1, new Date("2024-08-07T09:52:00.836Z")] as const;
const METHOD_BODY_COMBINATIONS = BODY_ACCEPTING_METHODS.flatMap(([method]) =>
BODIES.map((body) => [method, body] as const),
);

it.each(METHOD_BODY_COMBINATIONS)(
"implicit default content-type for body-full requests - %s, %j",
async (method, body) => {
const contentType = await fireRequestAndGetContentType({
method,
fetchOptions: {
body,
},
});

expect(contentType).toBe("application/json");
},
);

it.each(METHOD_BODY_COMBINATIONS)(
"provided default content-type for body-full requests - %s, %j",
async (method, body) => {
const contentType = await fireRequestAndGetContentType({
defaultHeaders: {
"content-type": "application/my-json",
},
method,
fetchOptions: {
body,
},
});

expect(contentType).toBe("application/my-json");
},
);

it.each(METHOD_BODY_COMBINATIONS)(
"native-fetch default content-type for body-full requests, when default is suppressed - %s, %j",
async (method, body) => {
const contentType = await fireRequestAndGetContentType({
defaultHeaders: {
"content-type": null,
},
method,
fetchOptions: {
body,
},
});
// the fetch implementation won't allow sending a body without content-type,
// and it defaults to `text/plain;charset=UTF-8`, however the actual default value
// is irrelevant and might be flaky across different fetch implementations
// for us, it's important that it's not `application/json`
expect(contentType).not.toBe("application/json");
},
);

it.each(METHOD_BODY_COMBINATIONS)(
"specified content-type for body-full requests - %s, %j",
async (method, body) => {
const contentType = await fireRequestAndGetContentType({
method,
fetchOptions: {
body,
headers: {
"content-type": "application/my-json",
},
},
});

expect(contentType).toBe("application/my-json");
},
);

it.each(METHOD_BODY_COMBINATIONS)(
"specified content-type for body-full requests, even when default is suppressed - %s, %j",
async (method, body) => {
const contentType = await fireRequestAndGetContentType({
method,
fetchOptions: {
body,
headers: {
"content-type": "application/my-json",
},
},
});

expect(contentType).toBe("application/my-json");
},
);
});

describe("fetch", () => {
it("createClient", async () => {
function createCustomFetch(data: any) {
Expand Down

0 comments on commit b893c44

Please sign in to comment.