Resolve message endpoint path using request context path

[tkpara]

When HttpServletSseServerTransportProvider handling the GET request to establish a new SSE connection, the message endpoint is advertised using the immutable field "baseUrl" that's set in the constructor:

// Send initial endpoint event this.sendEvent(writer, ENDPOINT_EVENT_TYPE, this.baseUrl + this.messageEndpoint + "?sessionId=" + sessionId);

This will not work if clients use a different base URL to access this servlet (e.g.: using reverse proxies). The servlet implementation should allow the request context path to be used to produce the endpoint path appropriate for that request:

// Send initial endpoint event this.sendEvent(writer, ENDPOINT_EVENT_TYPE, request.getContextPath() + this.messageEndpoint + "?sessionId=" + sessionId);

[denniskawurek]

Hi @tkpara , maybe your issue is related to my PR: #262

[tkpara]

@denniskawurek The PR doesn't seem to be related. The paths are still defined solely on server-side configurations and does not address the use case in which the client uses different paths to access these endpoints.

[saumyapandyaa]

I'd like to work on this one. Before I go reproduce it and put a PR together, here's where my head's at so far.

Looked through HttpServletSseServerTransportProvider and baseUrl is only actually used in buildEndpointUrl(String sessionId). That's called from one spot, doGet, and HttpServletRequest is already available right there. So I don't think this needs any new add-ons, just swap what gets passed in.

What I'm thinking:

• change buildEndpointUrl(String sessionId) to take a contextPath too
• call it with request.getContextPath() from doGet
• based on the fix in the issue description, this replaces baseUrl rather than combining with it.

Before writing the actual diff I want to check a couple things by actually running it:

• what happens when getContextPath() is empty (root deployments) - does the URL still come out right
• whether baseUrl / the builder's baseUrl(...) setter still needs to exist for anything else, or if this makes it dead for endpoint construction

From reading it, doPost and stdio transport don't touch this method at all, so this should stay pretty contained. Will confirm that as well.

Going to add tests for the different-context-path case, the empty-context-path case, and check if any existing tests assume the old baseUrl behavior.