Skip to content

check the the header X-Forwarded-Proto for scheme #14

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 3 commits into from
Jun 12, 2025
Merged

check the the header X-Forwarded-Proto for scheme #14

merged 3 commits into from
Jun 12, 2025

Conversation

@bangnokia
Copy link
Contributor

@bangnokia bangnokia commented Jun 9, 2025

Atm, when request to the /mcp/sse endoint via https (nginx proxy to the http transport server), it not respect the X-Forwarded-Proto in the header.

Hi, this is just a idea I made to make it work, I'm not sure how to proper handle the request scheme here

This is one sample from my app

curl -v https://mcp.daudau.cc/mcp/sse
* Host mcp.daudau.cc:443 was resolved.
* IPv6: (none)
* IPv4: 209.126.87.167
*   Trying 209.126.87.167:443...
* Connected to mcp.daudau.cc (209.126.87.167) port 443
* ALPN: curl offers h2,http/1.1
* (304) (OUT), TLS handshake, Client hello (1):
*  CAfile: /etc/ssl/cert.pem
*  CApath: none
* (304) (IN), TLS handshake, Server hello (2):
* (304) (IN), TLS handshake, Unknown (8):
* (304) (IN), TLS handshake, Certificate (11):
* (304) (IN), TLS handshake, CERT verify (15):
* (304) (IN), TLS handshake, Finished (20):
* (304) (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / AEAD-AES256-GCM-SHA384 / [blank] / UNDEF
* ALPN: server accepted h2
* Server certificate:
*  subject: CN=mcp.daudau.cc
*  start date: Jun  9 09:05:53 2025 GMT
*  expire date: Sep  7 09:05:52 2025 GMT
*  subjectAltName: host "mcp.daudau.cc" matched cert's "mcp.daudau.cc"
*  issuer: C=US; O=Let's Encrypt; CN=E5
*  SSL certificate verify ok.
* using HTTP/2
* [HTTP/2] [1] OPENED stream for https://mcp.daudau.cc/mcp/sse
* [HTTP/2] [1] [:method: GET]
* [HTTP/2] [1] [:scheme: https]
* [HTTP/2] [1] [:authority: mcp.daudau.cc]
* [HTTP/2] [1] [:path: /mcp/sse]
* [HTTP/2] [1] [user-agent: curl/8.7.1]
* [HTTP/2] [1] [accept: */*]
> GET /mcp/sse HTTP/2
> Host: mcp.daudau.cc
> User-Agent: curl/8.7.1
> Accept: */*
>
* Request completely sent off
< HTTP/2 200
< server: nginx
< date: Mon, 09 Jun 2025 11:34:40 GMT
< content-type: text/event-stream
< cache-control: no-cache
< access-control-allow-origin: *
< strict-transport-security: max-age=63072000
< x-frame-options: SAMEORIGIN
< x-xss-protection: 1; mode=block
< x-content-type-options: nosniff
<
event: endpoint
id: init-sse_878e3e14eca21477a19082a458d393b6
data: http://mcp.daudau.cc/mcp/message?clientId=sse_878e3e14eca21477a19082a458d393b6

@CodeWithKyrian
Copy link
Contributor

CodeWithKyrian commented Jun 12, 2025

Hi @bangnokia, thanks for this PR and for identifying the issue with the endpoint URL scheme when running behind an HTTPS reverse proxy.

After re-evaluating how clients typically resolve SSE event data (as per the MCP spec), I think there's an even simpler and more robust approach that avoids relying on forwarded headers directly in the server for URL generation.

MCP clients (and browsers/HTTP clients in general) will resolve path-absolute URLs (those starting with a /) relative to the origin (scheme + host + port) of the current connection.

So, instead of the server trying to reconstruct the full absolute URL for the endpoint event, we can simply send the path-absolute string for the message endpoint. The client will then correctly combine it with the origin it used to establish the SSE connection.

This would involve changing the endpoint event data generation in HttpServerTransport.php from constructing a full URI object to just sending the path.

Current (simplified):

$baseUri = $request->getUri()->withPath($this->messagePath) ... ;
$postEndpointWithId = (string) $baseUri->withQuery("clientId={$sessionId}");
$this->sendSseEvent($sseStream, 'endpoint', $postEndpointWithId, ...);

Proposed Change:

// $this->messagePath is already something like "/mcp_prefix/message"
$postEndpoint = $this->messagePath . "?clientId=" . $clientId;
$this->sendSseEvent($sseStream, 'endpoint', $postEndpoint, "init-{$clientId}");

Would you mind updating your PR to implement this approach or something similar?

@CodeWithKyrian CodeWithKyrian marked this pull request as draft June 12, 2025 08:12
@bangnokia
Copy link
Contributor Author

bangnokia commented Jun 12, 2025

@CodeWithKyrian thank you, I got it. Just updated my change for simplify

@CodeWithKyrian CodeWithKyrian marked this pull request as ready for review June 12, 2025 12:11
@CodeWithKyrian CodeWithKyrian merged commit afb1320 into php-mcp:main Jun 12, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bangnokia @CodeWithKyrian