Skip to content

add custom headers on initial _startOrAuth call #318

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 9 commits into from
Jul 7, 2025
Merged

add custom headers on initial _startOrAuth call #318

merged 9 commits into from
Jul 7, 2025

Conversation

anthonjn
Copy link
Contributor

@anthonjn anthonjn commented Apr 12, 2025 •
edited
Loading

Ensure that custom headers provided in this._requestInit?.headers are included in the initial call to the /sse endpoint.

Motivation and Context

Fixes #317, where custom headers (e.g. Host, X-*) were not sent in the initial /sse request, but were correctly sent in /messages. This is needed for MCP servers behind proxies that rely on these headers.

This PR aligns the behavior of the /sse request with send() by ensuring that all custom headers (when provided) are forwarded appropriately.

How Has This Been Tested?

  • Added a test case to verify that custom headers, when specified, are included in the initial /sse request.

Breaking Changes

No

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

N/a

@anthonjn anthonjn mentioned this pull request Apr 12, 2025
Closed
@cnjsstong
Copy link

This fix for requestInit.headers not working bug looks good. Please review and merge. Thanks!

@autoexpect
Copy link

@ihrpr Could you please help with the merge? Thanks!

Pizzaface pushed a commit to RewstApp/mcp-inspector that referenced this pull request May 2, 2025
@cliffhall
Merge pull request modelcontextprotocol#318 from lcamhoa/update-readme
214e5e4 
Update README requirement node js version
@ihrpr ihrpr added this to 🐛 🛠 May 29, 2025
@github-project-automation github-project-automation bot moved this to To triage in 🐛 🛠 May 29, 2025
@ihrpr ihrpr added this to the auth milestone Jun 25, 2025
@ochafik ochafik self-assigned this Jun 30, 2025
@ochafik ochafik self-requested a review June 30, 2025 10:46
@ochafik ochafik removed their assignment Jun 30, 2025
ochafik
ochafik requested changes Jun 30, 2025
View reviewed changes
Copy link
Contributor

@ochafik ochafik left a comment

Choose a reason for hiding this comment

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

Thanks @anthonjn, a much needed update indeed! Suggested a slight refactor :-)

@ochafik ochafik merged commit 6ae4a57 into modelcontextprotocol:main Jul 7, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from To triage to Done in 🐛 🛠 Jul 7, 2025
@anthonjn
Copy link
Contributor Author

anthonjn commented Jul 7, 2025

Missed the first comment notification, thanks @ochafik for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ochafik ochafik ochafik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
🐛 🛠
Status: Done
Milestone
auth
Development

Successfully merging this pull request may close these issues.

SSEClientTransport's custom header support seems inconsistent
5 participants
@anthonjn @cnjsstong @autoexpect @ochafik @ihrpr