Merge branch 'v2/main' into v2/fix/2123-root-union-schemas

This commit is contained in:
Cliff Hall 2026-08-26 00:54:25 -04:00 • committed by GitHub
commit 67d612a7eb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 578 additions and 56 deletions

View file

@ -1,9 +1,6 @@
import {
discoverAuthorizationServerMetadata,
discoverOAuthProtectedResourceMetadata,
} from "@modelcontextprotocol/client";
import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/client";
import type { OAuthClientInformation } from "@modelcontextprotocol/client";
import { getAuthorizationServerUrl } from "./discovery.js";
import { discoverAuthorizationServerMetadataForServer } from "./discovery.js";
import type { BaseOAuthClientProvider } from "./providers.js";
/**
@ -49,14 +46,14 @@ export async function ensureCimdClientRegistration(params: {
resourceMetadata = undefined;
}
const authServerUrl = getAuthorizationServerUrl(
// Walks the path-scoped authorization-server URL before the bare origin, so a
// server hosted under a path is probed where it actually publishes its
// metadata rather than only at the domain root (#2110).
const metadata = await discoverAuthorizationServerMetadataForServer(
params.serverUrl,
resourceMetadata,
params.fetchFn,
);
const metadata = await discoverAuthorizationServerMetadata(authServerUrl, {
...(params.fetchFn && { fetchFn: params.fetchFn }),
});
if (!metadata?.client_id_metadata_document_supported) return;
const clientInformation: OAuthClientInformation = {

View file

@ -2,6 +2,38 @@ import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/clien
import type { OAuthProtectedResourceMetadata } from "@modelcontextprotocol/client";
import { parseHttpUrl } from "./utils.js";
type AuthorizationServerMetadata = Awaited<
ReturnType<typeof discoverAuthorizationServerMetadata>
>;
/**
* The MCP server's own URL, used as the authorization server when RFC 9728
* protected-resource metadata named none.
*
* The path is kept. Reducing it to the origin (`new URL("/", serverUrl)`)
* discards exactly the part that tells discovery where to look when the server
* is not hosted at the domain root: the SDK's `buildDiscoveryUrls` derives the
* path-scoped well-known locations — `/.well-known/oauth-authorization-server{path}`
* and `{path}/.well-known/openid-configuration` — from this URL's pathname, so
* an origin-only value can only ever probe the domain root (#2110).
*
* Query and fragment are stripped: an RFC 8414 issuer identifier carries
* neither, and the SDK compares the discovered `issuer` against this URL.
*/
function serverUrlAsAuthorizationServer(serverUrl: string): URL {
try {
const url = new URL(serverUrl);
url.search = "";
url.hash = "";
return url;
} catch (err) {
const detail = err instanceof Error ? err.message : String(err);
throw new Error(`Invalid MCP server URL: "${serverUrl}" (${detail})`, {
cause: err,
});
}
}
/**
* Returns the URL to use for OAuth authorization server metadata discovery.
* Uses resource metadata's authorization_servers[0] when present, otherwise the MCP server URL.
@ -15,14 +47,87 @@ export function getAuthorizationServerUrl(
if (first) {
return parseHttpUrl(first, "protected resource authorization_servers[0]");
}
try {
return new URL("/", serverUrl);
} catch (err) {
const detail = err instanceof Error ? err.message : String(err);
throw new Error(`Invalid MCP server URL: "${serverUrl}" (${detail})`, {
cause: err,
});
return serverUrlAsAuthorizationServer(serverUrl);
}
/**
* The authorization-server URLs to try, in order.
*
* When protected-resource metadata names an authorization server there is
* exactly one candidate — the server told us where to look, and probing past it
* would be guessing. Otherwise the MCP server URL itself stands in as the
* authorization server, and a path-hosted server has two plausible answers: the
* full URL, whose path-scoped well-known locations are what a server like
* `https://example.com/mod/minilesson/mcp.php` publishes, and the bare origin,
* used by a server that merely *lives* under a path while publishing its
* metadata at the domain root. Trying the path-scoped form first fixes #2110
* without regressing the root-hosted case, which is all the origin-only value
* ever served.
*/
export function getAuthorizationServerUrlCandidates(
serverUrl: string,
resourceMetadata?: OAuthProtectedResourceMetadata | null,
): URL[] {
const primary = getAuthorizationServerUrl(serverUrl, resourceMetadata);
const namedByMetadata = Boolean(resourceMetadata?.authorization_servers?.[0]);
if (namedByMetadata || primary.pathname === "/") return [primary];
return [primary, new URL("/", primary)];
}
export interface DiscoveredAuthorizationServer {
/** The candidate that answered — not necessarily the first one tried. */
authorizationServerUrl: URL;
metadata: NonNullable<AuthorizationServerMetadata>;
}
/**
* Walks `candidates` until one answers, and reports **which** one did.
*
* A candidate that fails — a 5xx, or an RFC 8414 §3.3 issuer mismatch, both of
* which the SDK raises rather than returning `undefined` — is not fatal while
* another candidate remains. If every candidate fails, the first error is
* rethrown, so a caller still sees the original diagnosis rather than a bare
* `undefined`.
*
* `discover` is injected rather than closed over so a caller that already owns
* a discovery function — the CLI's `refreshStoredAuthToken`, which takes one as
* a test seam — can walk the same candidates without giving that seam up.
*/
export async function discoverAuthorizationServerMetadataFromCandidates(
candidates: URL[],
discover: (url: URL) => Promise<AuthorizationServerMetadata>,
): Promise<DiscoveredAuthorizationServer | undefined> {
let firstError: unknown;
let sawError = false;
for (const candidate of candidates) {
try {
const metadata = await discover(candidate);
if (metadata) return { authorizationServerUrl: candidate, metadata };
} catch (err) {
if (!sawError) {
firstError = err;
sawError = true;
}
}
}
if (sawError) throw firstError;
return undefined;
}
/**
* Discovers authorization server metadata for an MCP server, walking the
* candidates above until one answers.
*/
export async function discoverAuthorizationServerMetadataForServer(
serverUrl: string,
resourceMetadata?: OAuthProtectedResourceMetadata | null,
fetchFn?: typeof fetch,
): Promise<AuthorizationServerMetadata> {
const found = await discoverAuthorizationServerMetadataFromCandidates(
getAuthorizationServerUrlCandidates(serverUrl, resourceMetadata),
(candidate) => discoverAuthorizationServerMetadata(candidate, { fetchFn }),
);
return found?.metadata;
}
/**
@ -38,13 +143,11 @@ export const discoverScopes = async (
fetchFn?: typeof fetch,
): Promise<string | undefined> => {
try {
const authServerUrl = getAuthorizationServerUrl(
const metadata = await discoverAuthorizationServerMetadataForServer(
serverUrl,
resourceMetadata,
);
const metadata = await discoverAuthorizationServerMetadata(authServerUrl, {
fetchFn,
});
);
// Prefer resource metadata scopes, but fall back to OAuth metadata if empty
const resourceScopes = resourceMetadata?.scopes_supported;

View file

@ -165,14 +165,55 @@ function legacySessionId(json: RemoteConnectResponse): string | undefined {
}
/**
* Parse SSE stream from a ReadableStream.
* Yields { event, data } for each SSE message.
* Parse an SSE stream from a ReadableStream.
* Yields `{ event, data }` for each complete SSE message.
*
* ⚠️ **The in-progress event must live OUTSIDE the read loop.** A `read()`
* returns an arbitrary slice of bytes, not a whole frame, so a frame's `data:`
* line can arrive in one chunk and the blank line terminating it in the next.
* Declaring `currentEvent` / `currentData` inside the loop — which this did
* until #2134 — discards the accumulated payload at that boundary: the frame is
* never yielded, the JSON-RPC response it carried never settles, and the caller
* hangs forever with no error on any channel. `buffer` already carries a partial
* *line* across reads; these carry the partial *frame*.
*
* It survived because Chromium and Firefox happen to deliver whole frames at the
* payload sizes this transport sees. That is luck rather than a guarantee —
* chunk boundaries are a function of payload size, TCP segmentation and any
* intermediary — and the exposure grows with payload size, so a large
* `resources/read` is the most likely first victim.
*
* Exported for `clients/web/src/test/core/mcp/remote/parseSSE.test.ts`, which is
* the only way to reach the defect deterministically: driving a real transport
* cannot steer where the chunk boundary lands, but a test that supplies the
* reader can put it exactly on the terminator.
*/
async function* parseSSE(
export async function* parseSSE(
reader: ReadableStreamDefaultReader<Uint8Array>,
): AsyncGenerator<{ event: string; data: string }> {
const decoder = new TextDecoder();
let buffer = "";
// Carried ACROSS reads, not per chunk — see the header.
let currentEvent = "message";
let currentData: string[] = [];
const consume = (line: string): { event: string; data: string } | null => {
if (line.startsWith("event:")) {
currentEvent = line.slice(6).trim();
} else if (line.startsWith("data:")) {
currentData.push(line.slice(5).trimStart());
} else if (line === "") {
const complete =
currentData.length > 0
? { event: currentEvent, data: currentData.join("\n") }
: null;
currentEvent = "message";
currentData = [];
return complete;
}
// Anything else (a `:` keep-alive comment, `id:`, `retry:`) is ignored.
return null;
};
while (true) {
const { done, value } = await reader.read();
@ -183,36 +224,21 @@ async function* parseSSE(
/* v8 ignore next -- String.split always returns a non-empty array, so pop() is never undefined; the ?? "" fallback is unreachable. */
buffer = lines.pop() ?? "";
let currentEvent = "message";
let currentData: string[] = [];
for (const line of lines) {
if (line.startsWith("event:")) {
currentEvent = line.slice(6).trim();
} else if (line.startsWith("data:")) {
currentData.push(line.slice(5).trimStart());
} else if (line === "") {
if (currentData.length > 0) {
yield { event: currentEvent, data: currentData.join("\n") };
}
currentEvent = "message";
currentData = [];
}
const complete = consume(line);
if (complete) yield complete;
}
}
if (buffer.trim()) {
const lines = buffer.split("\n");
let currentEvent = "message";
const currentData: string[] = [];
for (const line of lines) {
if (line.startsWith("event:")) currentEvent = line.slice(6).trim();
else if (line.startsWith("data:"))
currentData.push(line.slice(5).trimStart());
}
if (currentData.length > 0) {
yield { event: currentEvent, data: currentData.join("\n") };
}
// Stream ended: flush the trailing partial line, then anything still held. A
// server that closes without a final blank line has still delivered a complete
// frame, and dropping it would lose the last message of every such stream.
if (buffer) {
const complete = consume(buffer);
if (complete) yield complete;
}
if (currentData.length > 0) {
yield { event: currentEvent, data: currentData.join("\n") };
}
}