Include requestInit options in GET SSE stream request#1592
Open
bhosmer-ant wants to merge 1 commit intomainfrom
Open
Include requestInit options in GET SSE stream request#1592bhosmer-ant wants to merge 1 commit intomainfrom
bhosmer-ant wants to merge 1 commit intomainfrom
Conversation
The GET request in _startOrAuthSse() did not spread this._requestInit into
the fetch options, unlike POST (send()) and DELETE (terminateSession()).
While _commonHeaders() does extract headers from requestInit.headers,
non-header fetch options like credentials, mode, cache, and referrerPolicy
were lost. This meant users configuring requestInit: { credentials: 'include' }
for cookie-based auth would have POST/DELETE send cookies but the GET SSE
stream would not.
Supersedes #896 (stale against pre-monorepo file layout).
Resolves #895
Co-authored-by: De Wildt van Reenen <dewildt@labs.epiuse.com>
|
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The GET request in
_startOrAuthSse()did not spreadthis._requestInitinto the fetch options, unlike POST (send()) and DELETE (terminateSession()). While_commonHeaders()does extract headers fromrequestInit.headers, non-header fetch options likecredentials,mode,cache, andreferrerPolicywere dropped.This meant users configuring
requestInit: { credentials: 'include' }for cookie-based auth would have POST/DELETE send cookies but the GET SSE notification stream would not — causing auth failures on server-initiated messages.Resolves #895
Supersedes #896 — same fix, but that PR targets the pre-monorepo file layout and hasn't been rebased.
Motivation and Context
Users connecting to servers that use cookie-based authentication configure
requestInit: { credentials: 'include', mode: 'cors' }. This worked for all POST/DELETE requests but silently failed for the GET SSE stream, since non-headerRequestInitproperties were never applied there.How Has This Been Tested?
Added a unit test that configures
requestInit: { credentials: 'include', mode: 'cors' }, calls_startOrAuthSse(), and verifies the fetch was called withcredentials: 'include'andmode: 'cors'.Breaking Changes
None. This brings the GET request into consistency with the existing POST/DELETE behavior.
Types of changes
Checklist
Additional context
Credit to @DW8Reaper for the original fix in #896.