Skip to content

[fix] Fix handling for Streams in IConnection for raw content#2791

Merged
kfcampbell merged 3 commits intooctokit:mainfrom
martincostello:fix-stream-handling
Nov 7, 2023
Merged

[fix] Fix handling for Streams in IConnection for raw content#2791
kfcampbell merged 3 commits intooctokit:mainfrom
martincostello:fix-stream-handling

Conversation

@martincostello
Copy link
Copy Markdown
Contributor

Fix NullReferenceException if raw content was handled as a string rather than a stream by HttpClientAdapter.BuildResponse().

I haven't added a test for this, but have validated that the apps that surfaced this issue are working when using a local build of Octokit including this change. If you can direct me to exactly where you'd like a test for this, I'll add one.

Resolves #2789


Before the change?

  • A NullReferenceException would be thrown.

After the change?

  • The call succeeds.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

  • Yes
  • No

@martincostello martincostello mentioned this pull request Oct 2, 2023
4 tasks
@nickfloyd nickfloyd added the Type: Bug Something isn't working as documented label Oct 2, 2023
@nickfloyd nickfloyd closed this in 12e29ab Oct 3, 2023
@martincostello martincostello deleted the fix-stream-handling branch October 3, 2023 17:11
@martincostello martincostello restored the fix-stream-handling branch October 3, 2023 17:12
@martincostello
Copy link
Copy Markdown
Contributor Author

Looks like this got closed by accident because I put the wrong # in the commit message for the other PR, and I can't re-open it.

@martincostello
Copy link
Copy Markdown
Contributor Author

@nickfloyd Anything more you need from me to progress this fix?

@kfcampbell
Copy link
Copy Markdown
Contributor

I do think it'd be possible to write a test for this case in Octokit.Tests/Http/ConnectionTests.cs, since the modified GetRaw is called by two public functions. Why don't we try doing so?

martincostello and others added 3 commits November 1, 2023 09:38
Fix `NullReferenceException` if raw content was handled as a string rather than a stream by `HttpClientAdapter.BuildResponse()`.
Resolves #2789.
Add tests for `Connection.GetRaw()` and for #2789.
Copy link
Copy Markdown
Contributor

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

@kfcampbell kfcampbell merged commit 4f826bc into octokit:main Nov 7, 2023
@martincostello martincostello deleted the fix-stream-handling branch November 7, 2023 06:56
Lulalaby referenced this pull request in Fortunevale/ProjectMakoto Jan 3, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [Octokit](https://togithub.com/octokit/octokit.net) | `9.0.0` ->
`9.1.0` |
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Octokit/9.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Octokit/9.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Octokit/9.0.0/9.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Octokit/9.0.0/9.1.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>octokit/octokit.net (Octokit)</summary>

###
[`v9.1.0`](https://togithub.com/octokit/octokit.net/releases/tag/v9.1.0)

#### What's Changed

##### Features

- \[feat]: Output GraphQL rate limit by
[@&#8203;dirtygooback](https://togithub.com/dirtygooback) in
[https://github.com/octokit/octokit.net/pull/2807](https://togithub.com/octokit/octokit.net/pull/2807)
- \[feat]: Allow the constructor for `ProductHeaderValue` to accept a
framework version of the object by
[@&#8203;Cyberboss](https://togithub.com/Cyberboss) in
[https://github.com/octokit/octokit.net/pull/2821](https://togithub.com/octokit/octokit.net/pull/2821)
- \[feat] Introduce NewRelease.DiscussionCategoryName, Fixes
[#&#8203;2746](https://togithub.com/octokit/octokit.net/issues/2746) by
[@&#8203;AndreyAkinshin](https://togithub.com/AndreyAkinshin) in
[https://github.com/octokit/octokit.net/pull/2842](https://togithub.com/octokit/octokit.net/pull/2842)
- \[feat]: Adding Copilot for Business support by
[@&#8203;dylan-asos](https://togithub.com/dylan-asos) in
[https://github.com/octokit/octokit.net/pull/2826](https://togithub.com/octokit/octokit.net/pull/2826)
- \[feat]: Implement Actions OIDC Client by
[@&#8203;dirtygooback](https://togithub.com/dirtygooback) in
[https://github.com/octokit/octokit.net/pull/2828](https://togithub.com/octokit/octokit.net/pull/2828)
- \[feat] Add support for "Require approval of the most recent
reviewable push" by [@&#8203;ChrisAnn](https://togithub.com/ChrisAnn) in
[https://github.com/octokit/octokit.net/pull/2839](https://togithub.com/octokit/octokit.net/pull/2839)

##### Bugs

- \[fix] Fix handling for Streams in IConnection for raw content by
[@&#8203;martincostello](https://togithub.com/martincostello) in
[https://github.com/octokit/octokit.net/pull/2791](https://togithub.com/octokit/octokit.net/pull/2791)
- \[fix]: The methods `GetAllForCurrentWithTimestamps` and
`GetAllForUserWithTimestamps` did not return timestamps. by
[@&#8203;MareMare](https://togithub.com/MareMare) in
[https://github.com/octokit/octokit.net/pull/2829](https://togithub.com/octokit/octokit.net/pull/2829)

##### Maintenance

- build(deps): bump Microsoft.NET.Test.Sdk from 17.7.2 to 17.8.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/octokit/octokit.net/pull/2811](https://togithub.com/octokit/octokit.net/pull/2811)
- build(deps): bump Microsoft.SourceLink.GitHub from 1.1.1 to 8.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/octokit/octokit.net/pull/2816](https://togithub.com/octokit/octokit.net/pull/2816)
- build(deps): bump Microsoft.Bcl.AsyncInterfaces from 7.0.0 to 8.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/octokit/octokit.net/pull/2817](https://togithub.com/octokit/octokit.net/pull/2817)
- build(deps): bump Cake.Frosting from 2.3.0 to 4.0.0 in /build by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/octokit/octokit.net/pull/2814](https://togithub.com/octokit/octokit.net/pull/2814)
- build(deps): bump xunit from 2.6.3 to 2.6.4 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/octokit/octokit.net/pull/2841](https://togithub.com/octokit/octokit.net/pull/2841)
- build(deps): bump xunit.runner.visualstudio from 2.5.5 to 2.5.6 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/octokit/octokit.net/pull/2840](https://togithub.com/octokit/octokit.net/pull/2840)
- \[maint]: don't run immediate response on Actions- or
octokitbot-driven changes by
[@&#8203;octokitbot](https://togithub.com/octokitbot) in
[https://github.com/octokit/octokit.net/pull/2819](https://togithub.com/octokit/octokit.net/pull/2819)
- \[maint]: Workflows have changed by
[@&#8203;octokitbot](https://togithub.com/octokitbot) in
[https://github.com/octokit/octokit.net/pull/2830](https://togithub.com/octokit/octokit.net/pull/2830)

#### New Contributors

- [@&#8203;AndreyAkinshin](https://togithub.com/AndreyAkinshin) made
their first contribution in
[https://github.com/octokit/octokit.net/pull/2842](https://togithub.com/octokit/octokit.net/pull/2842)
- [@&#8203;MareMare](https://togithub.com/MareMare) made their first
contribution in
[https://github.com/octokit/octokit.net/pull/2829](https://togithub.com/octokit/octokit.net/pull/2829)
- [@&#8203;ChrisAnn](https://togithub.com/ChrisAnn) made their first
contribution in
[https://github.com/octokit/octokit.net/pull/2839](https://togithub.com/octokit/octokit.net/pull/2839)

**Full Changelog**:
octokit/octokit.net@v9.0.0...v9.1.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/Fortunevale/ProjectMakoto).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMDMuMSIsInVwZGF0ZWRJblZlciI6IjM3LjEwMy4xIiwidGFyZ2V0QnJhbmNoIjoiZGV2In0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Bug Something isn't working as documented

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[BUG]: NullReferenceException thrown using IConnection.GetRaw() for JSON content in Octokit 8.0.1

3 participants