From 51ba27c457d482a29684338c525a09b5360f9c74 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Thu, 16 Jan 2025 15:59:02 +0000 Subject: [PATCH 1/5] Correctly parse response bodies as JSON where the Content-Type is `application/scim+json` GitHub has APIs that return `application/scim+json` response bodies. Currently, these responses are not parsed as JSON, and instead, an `ArrayBuffer` is returned in `response.data`. This adds handling for `application/scim+json` so they are parsed as JSON as normal. ## Minimal reproduction ```js import { Octokit } from "@octokit/rest"; import { enterpriseCloud } from "@octokit/plugin-enterprise-cloud"; const MyOctokit = Octokit.plugin(enterpriseCloud); const myOctokit = new MyOctokit({ auth: "ghp_xxxxxxxx", }); const resp = await myOctokit.request('GET /scim/v2/enterprises/{enterprise}/Groups', { enterprise: 'fabrikam' }); console.log(JSON.stringify(resp.data)); ## Detailed changes * Modify `getResponseData` function in `src/fetch-wrapper.ts` to include a check for `application/scim+json` content type. * Refactor content type check into a new `isJSONResponse` function. * Add a test in `test/request.test.ts` to verify that response bodies with `application/scim+json` content type are correctly parsed as JSON. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/octokit/request.js?shareId=XXXX-XXXX-XXXX-XXXX). --- src/fetch-wrapper.ts | 6 +++++- test/request.test.ts | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/fetch-wrapper.ts b/src/fetch-wrapper.ts index a1596716e..3b3a4b878 100644 --- a/src/fetch-wrapper.ts +++ b/src/fetch-wrapper.ts @@ -155,7 +155,7 @@ async function getResponseData(response: Response): Promise { const mimetype = safeParse(contentType); - if (mimetype.type === "application/json") { + if (isJSONResponse(mimetype)) { let text = ""; try { text = await response.text(); @@ -173,6 +173,10 @@ async function getResponseData(response: Response): Promise { } } +function isJSONResponse(mimetype: { type: string }) { + return mimetype.type === "application/json" || mimetype.type === "application/scim+json"; +} + function toErrorMessage(data: string | ArrayBuffer | Record) { if (typeof data === "string") { return data; diff --git a/test/request.test.ts b/test/request.test.ts index e4bee928e..fd38e45b3 100644 --- a/test/request.test.ts +++ b/test/request.test.ts @@ -869,7 +869,7 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== required_pull_request_reviews: { required_approving_review_count: 1, dismiss_stale_reviews: true, - require_code_owner_reviews: true, + require_code owner_reviews: true, dismissal_restrictions: { users: [], teams: [] }, }, restrictions: { users: [], teams: [] }, @@ -1263,4 +1263,41 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== }), ).rejects.toHaveProperty("message", "Unknown error: {}"); }); + + it("parses response bodies as JSON when Content-Type is application/scim+json", async () => { + expect.assertions(1); + + const mock = fetchMock.createInstance(); + mock.get("https://api.github.com/scim/v2/Users", { + status: 200, + body: { + totalResults: 1, + Resources: [ + { + id: "123", + userName: "octocat", + }, + ], + }, + headers: { + "Content-Type": "application/scim+json", + }, + }); + + const response = await request("GET /scim/v2/Users", { + request: { + fetch: mock.fetchHandler, + }, + }); + + expect(response.data).toEqual({ + totalResults: 1, + Resources: [ + { + id: "123", + userName: "octocat", + }, + ], + }); + }); }); From 71a5af4594dde2e65ccd6556fdbc274e62d3a71c Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Thu, 16 Jan 2025 16:03:01 +0000 Subject: [PATCH 2/5] Fix stray space --- test/request.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/request.test.ts b/test/request.test.ts index fd38e45b3..ce8d73279 100644 --- a/test/request.test.ts +++ b/test/request.test.ts @@ -869,7 +869,7 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== required_pull_request_reviews: { required_approving_review_count: 1, dismiss_stale_reviews: true, - require_code owner_reviews: true, + require_code_owner_reviews: true, dismissal_restrictions: { users: [], teams: [] }, }, restrictions: { users: [], teams: [] }, From 4900a74f6e126fbfddaf152db13441e3be2a849a Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Thu, 16 Jan 2025 16:03:13 +0000 Subject: [PATCH 3/5] Format with ESLint and Prettier --- src/fetch-wrapper.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/fetch-wrapper.ts b/src/fetch-wrapper.ts index 3b3a4b878..f9808e47c 100644 --- a/src/fetch-wrapper.ts +++ b/src/fetch-wrapper.ts @@ -174,7 +174,10 @@ async function getResponseData(response: Response): Promise { } function isJSONResponse(mimetype: { type: string }) { - return mimetype.type === "application/json" || mimetype.type === "application/scim+json"; + return ( + mimetype.type === "application/json" || + mimetype.type === "application/scim+json" + ); } function toErrorMessage(data: string | ArrayBuffer | Record) { From fc612765468ff99e1dcb6ab4f722ca77fb2fa26e Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Thu, 16 Jan 2025 16:03:52 +0000 Subject: [PATCH 4/5] Add return type annotation --- src/fetch-wrapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fetch-wrapper.ts b/src/fetch-wrapper.ts index f9808e47c..236bb1fe0 100644 --- a/src/fetch-wrapper.ts +++ b/src/fetch-wrapper.ts @@ -173,7 +173,7 @@ async function getResponseData(response: Response): Promise { } } -function isJSONResponse(mimetype: { type: string }) { +function isJSONResponse(mimetype: { type: string }): boolean { return ( mimetype.type === "application/json" || mimetype.type === "application/scim+json" From b87ddce80c8b1a34b5a41693be98b2b00f511241 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Thu, 16 Jan 2025 16:05:01 +0000 Subject: [PATCH 5/5] Improve input type validation --- src/fetch-wrapper.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/fetch-wrapper.ts b/src/fetch-wrapper.ts index 236bb1fe0..2313cccbc 100644 --- a/src/fetch-wrapper.ts +++ b/src/fetch-wrapper.ts @@ -3,6 +3,8 @@ import { isPlainObject } from "./is-plain-object.js"; import { RequestError } from "@octokit/request-error"; import type { EndpointInterface, OctokitResponse } from "@octokit/types"; +type ContentType = ReturnType; + export default async function fetchWrapper( requestOptions: ReturnType, ): Promise> { @@ -173,7 +175,7 @@ async function getResponseData(response: Response): Promise { } } -function isJSONResponse(mimetype: { type: string }): boolean { +function isJSONResponse(mimetype: ContentType): boolean { return ( mimetype.type === "application/json" || mimetype.type === "application/scim+json"