Skip to content

Correctly parse response bodies as JSON where the Content-Type is application/scim+json #731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jan 16, 2025
Merged

Correctly parse response bodies as JSON where the Content-Type is application/scim+json #731

merged 5 commits into from
Jan 16, 2025
+47 −1

Conversation

timrogers
Copy link
Contributor

@timrogers timrogers commented Jan 16, 2025 •
edited
Loading

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

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.

@timrogers
Correctly parse response bodies as JSON where the Content-Type is `ap…
Loading
Loading status checks…
51ba27c 
…plication/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).
@github-actions GitHub Actions
Copy link

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

timrogers
timrogers commented Jan 16, 2025
View reviewed changes
@timrogers
Fix stray space
71a5af4
@timrogers
Format with ESLint and Prettier

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
4900a74
@timrogers
Add return type annotation

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
fc61276
@timrogers
Improve input type validation
Loading
Loading status checks…
b87ddce
@wolfy1339 wolfy1339 merged commit 00bf316 into octokit:main Jan 16, 2025
7 checks passed
@github-actions GitHub Actions
Copy link

🎉 This PR is included in version 9.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

newkdr added a commit to newkdr/vscode-gitlens that referenced this pull request Feb 13, 2025
@newkdr
[Snyk] Upgrade @octokit/request from 9.1.3 to 9.2.0 (#19)
02fef6f 
![snyk-top-banner](https://redirect.github.com/andygongea/OWASP-Benchmark/assets/818805/c518c423-16fe-447e-b67f-ad5a49b5d123)


<h3>Snyk has created this PR to upgrade @octokit/request from 9.1.3 to
9.2.0.</h3>

:information_source: Keep your dependencies up-to-date. This makes it
easier to fix existing vulnerabilities and to more quickly identify and
fix newly disclosed vulnerabilities when they affect your project.

<hr/>


- The recommended version is **2 versions** ahead of your current
version.

- The recommended version was released **a month ago**.



<details>
<summary><b>Release notes</b></summary>
<br/>
  <details>
    <summary>Package name: <b>@octokit/request</b></summary>
    <ul>
      <li>
<b>9.2.0</b> - <a
href="https://redirect.github.com/octokit/request.js/releases/tag/v9.2.0">2025-01-16</a></br><h1><a
href="https://redirect.github.com/octokit/request.js/compare/v9.1.4...v9.2.0">9.2.0</a>
(2025-01-16)</h1>
<h3>Features</h3>
<ul>
<li>correctly parse response bodies as JSON where the Content-Type is
<code>application/scim+json</code> (<a
href="https://redirect.github.com/octokit/request.js/issues/731"
data-hovercard-type="pull_request"
data-hovercard-url="/octokit/request.js/pull/731/hovercard">gitkraken#731</a>)
(<a
href="https://redirect.github.com/octokit/request.js/commit/00bf316136acf001344a5bacff906f1748e6c368">00bf316</a>)</li>
</ul>
      </li>
      <li>
<b>9.1.4</b> - <a
href="https://redirect.github.com/octokit/request.js/releases/tag/v9.1.4">2024-12-29</a></br><h2><a
href="https://redirect.github.com/octokit/request.js/compare/v9.1.3...v9.1.4">9.1.4</a>
(2024-12-29)</h2>
<h3>Bug Fixes</h3>
<ul>
<li><strong>deps:</strong> bump <code>@ octokit/types</code> to fix deno
compat (<a
href="https://redirect.github.com/octokit/request.js/issues/730"
data-hovercard-type="pull_request"
data-hovercard-url="/octokit/request.js/pull/730/hovercard">gitkraken#730</a>)
(<a
href="https://redirect.github.com/octokit/request.js/commit/324ffef20c305f9db4813b84518c40e2f3cd76b0">324ffef</a>)</li>
</ul>
      </li>
      <li>
<b>9.1.3</b> - <a
href="https://redirect.github.com/octokit/request.js/releases/tag/v9.1.3">2024-07-14</a></br><h2><a
href="https://redirect.github.com/octokit/request.js/compare/v9.1.2...v9.1.3">9.1.3</a>
(2024-07-14)</h2>
<h3>Bug Fixes</h3>
<ul>
<li>improve toErrorMessage (<a
href="https://redirect.github.com/octokit/request.js/issues/714"
data-hovercard-type="pull_request"
data-hovercard-url="/octokit/request.js/pull/714/hovercard">gitkraken#714</a>)
(<a
href="https://redirect.github.com/octokit/request.js/commit/fcc5b25e044a2dd0fb8c1a76390eb6da9aa06d42">fcc5b25</a>)</li>
</ul>
      </li>
    </ul>
from <a
href="https://redirect.github.com/octokit/request.js/releases">@octokit/request
GitHub release notes</a>
  </details>
</details>

---

> [!IMPORTANT]
>
> - Check the changes in this PR to ensure they won't cause issues with
your project.
> - This PR was automatically created by Snyk using the credentials of a
real user.

---

**Note:** _You are seeing this because you or someone else with access
to this repository has authorized Snyk to open upgrade PRs._

**For more information:** <img
src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiIwNTJmNjcxMi03NTY5LTQxZDAtODIwNi03NGVmMWRiMDQ2ZGEiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6IjA1MmY2NzEyLTc1NjktNDFkMC04MjA2LTc0ZWYxZGIwNDZkYSJ9fQ=="
width="0" height="0"/>

> - 🧐 [View latest project
report](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)
> - 📜 [Customise PR
templates](https://docs.snyk.io/scan-using-snyk/pull-requests/snyk-fix-pull-or-merge-requests/customize-pr-templates?utm_source=&utm_content=fix-pr-template)
> - 🛠 [Adjust upgrade PR
settings](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17/settings/integration?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)
> - 🔕 [Ignore this dependency or unsubscribe from future upgrade
PRs](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17/settings/integration?pkg&#x3D;@octokit/request&amp;utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr#auto-dep-upgrades)

[//]: #
'snyk:metadata:{"customTemplate":{"variablesUsed":[],"fieldsUsed":[]},"dependencies":[{"name":"@octokit/request","from":"9.1.3","to":"9.2.0"}],"env":"prod","hasFixes":false,"isBreakingChange":false,"isMajorUpgrade":false,"issuesToFix":[],"prId":"052f6712-7569-41d0-8206-74ef1db046da","prPublicId":"052f6712-7569-41d0-8206-74ef1db046da","packageManager":"npm","priorityScoreList":[],"projectPublicId":"12a8a5f5-3e19-438c-8280-eb8f4ee06d17","projectUrl":"https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17?utm_source=github&utm_medium=referral&page=upgrade-pr","prType":"upgrade","templateFieldSources":{"branchName":"default","commitMessage":"default","description":"default","title":"default"},"templateVariants":[],"type":"auto","upgrade":[],"upgradeInfo":{"versionsDiff":2,"publishedDate":"2025-01-16T16:54:38.296Z"},"vulns":[]}'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wolfy1339 wolfy1339

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@timrogers @wolfy1339