Skip to content

Remove UTF-8 charset parameter from Content-Type in SseEmitter #24632

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.

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

Closed

Conversation

sada-sigsci
Copy link
Contributor

SseEmitter is adding header "Content-type:text/event-stream;charset=UTF-8".
Golang revproxy is expecting header "Content-Type: text/event-stream" for
immediately flushing the event stream.

HTML standard says [ https://html.spec.whatwg.org/multipage/server-sent-events.html#sse-processing-model ]
ignore the MIME type param and the data is always expected in utf-8.

https://github.com/golang/go/blob/master/src/net/http/httputil/reverseproxy.go#L374

SseEmitter is adding header "Content-type:text/event-stream;charset=UTF-8".
Golang revproxy is expecting header "Content-Type: text/event-stream" for
immediately flushing the event stream.

HTML standard says [ https://html.spec.whatwg.org/multipage/server-sent-events.html#sse-processing-model ]
ignore the MIME type param and the data is always expected in utf-8.
@pivotal-issuemaster
Copy link

@sada-sigsci Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 3, 2020
@pivotal-issuemaster
Copy link

@sada-sigsci Thank you for signing the Contributor License Agreement!

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Mar 3, 2020

The origin of this seems to be #18978 which was added as a workaround for spring-projects/spring-boot#6230.

I don't entirely understand the original issue since the sample project repo is now gone. My assumption is that as a result of spring-projects/spring-boot#5459, use of spring.http.encoding.force-response=true started to set the charset on every response, and if that charset was ISO-8859-1 you'd get that also on SSE where it's wrong.

The workaround to add charset=UTF-8 to SSE responses does not seem to be free of side effects, as this issue shows, and we went through a similar cycle with removing the charset from application/json, see #22788.

I've accepted this for 5.2.5 but I'll allow some time for the Boot team to comment before processing this. /cc @bclozel @wilkinsona

@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement labels Mar 3, 2020
@rstoyanchev rstoyanchev added this to the 5.2.5 milestone Mar 3, 2020
@rstoyanchev rstoyanchev self-assigned this Mar 3, 2020
@rstoyanchev rstoyanchev removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 3, 2020
@wilkinsona
Copy link
Member

wilkinsona commented Mar 3, 2020

Thanks, @rstoyanchev. My memory of the Boot issue is hazy at best but I think the problem was reported when we were enforcing the encoding on both requests and responses. That was then split out so that it was only done for requests by default. If that's correct, you'd have to explicitly set spring.http.encoding.force-response=true to encounter the problem. As it was Jetty-specific, there may also be a bit more to it. I think we'd have to try and recreate the original problem to learn more.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Mar 3, 2020

Okay, I think we need to process this change to start. If that still causes the original side effect on Jetty with spring.http.encoding.force-response=true, we could call response.setCharacterEncoding(null) in the handling for SSE effectively undoing the forced charset, which causes issues for media types that are not expected to have a charset parameter.

@rstoyanchev rstoyanchev changed the title Remove MIME type param from content-type header Remove UTF-8 charset parameter from Content-Type in SseEmitter Mar 4, 2020
rstoyanchev pushed a commit that referenced this pull request Mar 4, 2020
rstoyanchev added a commit that referenced this pull request Mar 4, 2020
@sada-sigsci
Copy link
Contributor Author

Thank you so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants