Skip to content

fix: set "Content-Type" header rather than add#878

Open
jfoshee wants to merge 1 commit into
ory:masterfrom
jfoshee:patch-1
Open

fix: set "Content-Type" header rather than add#878
jfoshee wants to merge 1 commit into
ory:masterfrom
jfoshee:patch-1

Conversation

@jfoshee
Copy link
Copy Markdown

@jfoshee jfoshee commented May 19, 2026

in order to replace any header that was previously set on the response.

Multiple Content-Type headers are not supported by HTTP and can lead to unpredictable handling in the browser. In this particular case it can cause the html form to be rendered as plain text.

Related Issue or Design Document

Checklist

  • I have read the contributing guidelines and signed the CLA.
  • I have referenced an issue containing the design document if my change introduces a new feature.
  • I have read the security policy.
  • I confirm that this pull request does not address a security vulnerability.
    If this pull request addresses a security vulnerability,
    I confirm that I got approval (please contact security@ory.sh) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation within the code base (if appropriate).

Further comments

Summary by CodeRabbit

  • Bug Fixes
    • Fixed a header handling issue in form-based authorization responses where duplicate header values could be inadvertently set, potentially causing client-side processing errors and unexpected behavior. The correction ensures headers are set with a single, authoritative value, improving overall compatibility, reliability, and compliance with HTTP specifications.

Review Change Stack

in order to replace any header that was previously set on the response.

Multiple Content-Type headers are not supported by HTTP and can lead to unpredictable handling in the browser. In this particular case it can cause the html form to be rendered as plain text.
@jfoshee jfoshee requested review from a team and aeneasr as code owners May 19, 2026 16:09
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

📝 Walkthrough

Walkthrough

The PR fixes a header duplication issue in the OAuth2 authorization response handler. The WriteAuthorizeResponse function for ResponseModeFormPost now uses Header().Set instead of Header().Add when setting the Content-Type header, ensuring only a single value is present in the HTTP response.

Changes

Authorization Form Post Content-Type Header Fix

Layer / File(s) Summary
Form Post Response Mode Header Fix
authorize_write.go
The Content-Type header for form post authorization responses now uses Header().Set instead of Header().Add to prevent multiple header values from accumulating.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description explains the bug (multiple Content-Type headers cause browser rendering issues) and the fix, but lacks a linked issue reference as required by the template for bug fixes. Add a reference to the related issue using #1234 format, or clarify if this fix addresses a previously unknown bug that should be documented.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the main change: replacing Add with Set for the Content-Type header in the authorize_write.go file.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jfoshee jfoshee changed the title Set "Content-Type" header rather than Add fix(form_post): Set "Content-Type" header rather than Add May 19, 2026
@jfoshee jfoshee changed the title fix(form_post): Set "Content-Type" header rather than Add fix: Set "Content-Type" header rather than Add May 19, 2026
@jfoshee jfoshee changed the title fix: Set "Content-Type" header rather than Add fix: set "Content-Type" header rather than add May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants