Skip to content
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

1.4.0.M3: SseEmitter, Jetty, CORS: Wrong charset in Content-Type header #6230

Closed
ralscha opened this issue Jun 26, 2016 · 7 comments
Closed
Assignees
Labels
status: duplicate A duplicate of another issue

Comments

@ralscha
Copy link
Contributor

ralscha commented Jun 26, 2016

There is a problem in 1.4.0.M3 and 1.4.0.BUILD-SNAPSHOT, Jetty, EventSource (SseEmitter) and CORS. With this setup spring boot sends a wrong Content-Type response header back to the client.
Content-Type: text/event-stream; charset=ISO-8859-1

Chrome 51 complains with this error message
EventSource's response has a charset ("iso-8859-1") that is not UTF-8. Aborting the connection.

When the request is not sent with CORS the server sends this header
Content-Type: text/event-stream
which is okay for Chrome. Event streams must always be encoded using UTF-8

Here is a github repository that demonstrates the problem:
https://github.com/ralscha/sse-encoding

Requests to http://localhost:8080 work fine but when you do a CORS request (for example by opening index.html directly from filesystem) Chrome will complain with the above-mentioned error message.

This problem does not occur with Spring Boot 1.4.0.M2
and it does not occur with Tomcat and Undertow with 1.4.0.M3 and 1.4.0.BUILD-SNAPSHOT

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 26, 2016
@wilkinsona wilkinsona self-assigned this Jun 27, 2016
@wilkinsona
Copy link
Member

Thanks for the sample that reproduces the problem. The ISO-8859-1 response can be reproduced with a simple curl request:

curl -v localhost:8080/start -H 'Origin: foo'

@snicoll
Copy link
Member

snicoll commented Jun 27, 2016

Isn't that related to #5459 ?

@wilkinsona
Copy link
Member

I'm not sure yet. It seems likely, however I'd expect Tomcat and Undertow to be affected too.

@wilkinsona
Copy link
Member

When CORS is involved, DefaultCorsProcessor.handleInternal calls ServletServerHttpResponse.flush() which, in turn, calls ServletServerHttpResponse.writeHeaders(). As part of writing the headers, getCharacterEncoding() is called on the HttpServletResponse. Jetty's implementation is:

@Override
public String getCharacterEncoding()
{
    if (_characterEncoding == null)
        _characterEncoding = StringUtil.__ISO_8859_1;
    return _characterEncoding;
}

When the call is made, _characterEncoding is null so it's set to iso-8859-1. Without the Origin header, DefaultCorsProcessor doesn't really get involved, flush() isn't called, and the character encoding isn't initialized with the unwanted value.

@wilkinsona
Copy link
Member

As @snicoll suspected, this is a side-effect of #5459. In 1.4.0.M3 and later, responses can be set to UTF-8 by forcing the response's content type:

spring.http.encoding.force-response=true

The will set the charset of every response to UTF-8 which may or may not be desirable. If it's undesirable, one more focussed solution is a custom SseEmitter subclass. For example:

@Controller
public class SseController {

    @CrossOrigin
    @RequestMapping("/start")
    public SseEmitter start() {
        return new Utf8SseEmitter();
    }

    private static final class Utf8SseEmitter extends SseEmitter {

        private static final MediaType UTF8_TEXT_STREAM = new MediaType("text", "stream", Charset.forName("UTF-8"));

        @Override
        protected void extendResponse(ServerHttpResponse outputMessage) {
            HttpHeaders headers = outputMessage.getHeaders();
            if (headers.getContentType() == null) {
                headers.setContentType(UTF8_TEXT_STREAM);
            }
        }

    }

}

I'm not convinced that this should be necessary, though.

@bclozel Given that the spec states that text/stream must use UTF-8, would it make sense for SseEmitter to specify the UTF-8 charset by default as I have done above in Utf8SseEmitter?

@bclozel bclozel added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 27, 2016
@bclozel
Copy link
Member

bclozel commented Jun 27, 2016

This has been fixed in SPR-14407 and should be available shortly as a Spring Framework 4.3.1 SNAPSHOT version.

Thanks @wilkinsona !

@bclozel bclozel closed this as completed Jun 27, 2016
@snicoll snicoll removed the type: bug A general bug label Jun 27, 2016
@snicoll
Copy link
Member

snicoll commented Jun 27, 2016

Duplicates #6197

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

5 participants