Skip to content

Commit

Permalink
Include charset=utf-8 in the JSON response.
Browse files Browse the repository at this point in the history
Per spec, UTF-8 is the default, and the charset parameter should not
be necessary. But some clients (eg: Chrome) think otherwise.
Since json.Marshal produces UTF-8, setting the charset parameter is a
safe option.

This changeset also includes a better ContentTypeIsJson test method.
It expects the charset to be utf-8 or not set.
  • Loading branch information
ant0ine committed Nov 28, 2015
1 parent 1eb58eb commit 3152f30
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
6 changes: 5 additions & 1 deletion rest/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ type responseWriter struct {

func (w *responseWriter) WriteHeader(code int) {
if w.Header().Get("Content-Type") == "" {
w.Header().Set("Content-Type", "application/json")
// Per spec, UTF-8 is the default, and the charset parameter should not
// be necessary. But some clients (eg: Chrome) think otherwise.
// Since json.Marshal produces UTF-8, setting the charset parameter is a
// safe option.
w.Header().Set("Content-Type", "application/json; charset=utf-8")
}
w.ResponseWriter.WriteHeader(code)
w.wroteHeader = true
Expand Down
21 changes: 19 additions & 2 deletions rest/test/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
"mime"
"net/http"
"net/http/httptest"
"strings"
Expand Down Expand Up @@ -56,7 +57,23 @@ func HeaderIs(t *testing.T, r *httptest.ResponseRecorder, headerKey, expectedVal
}

func ContentTypeIsJson(t *testing.T, r *httptest.ResponseRecorder) {
HeaderIs(t, r, "Content-Type", "application/json")

mediaType, params, _ := mime.ParseMediaType(r.HeaderMap.Get("Content-Type"))
charset := params["charset"]

if mediaType != "application/json" {
t.Errorf(
"Content-Type media type: application/json expected, got: %s",
mediaType,
)
}

if charset != "" && strings.ToUpper(charset) != "UTF-8" {
t.Errorf(
"Content-Type charset: utf-8 or no charset expected, got: %s",
charset,

This comment has been minimized.

Copy link
@wichert

wichert Nov 30, 2015

The test does not seem to match the error message: if the charset is empty no error is produced. This should probably be charset == "" || strings.ToUpper(charset) != "UTF-8".

This comment has been minimized.

Copy link
@ant0ine

ant0ine Dec 1, 2015

Author Owner

My Go may be better than my english ;)
The error case is a charset explicitly specified AND that is not UTF-8. The valid case is the negation of it (ie: a charset NOT specified, OR specified as UTF-8.) Is the error message confusing ?

This comment has been minimized.

Copy link
@wichert

wichert Dec 3, 2015

Perhaps rephrase like "Content-Type charset: must be empty or UTF-8, got: %s" ?

This comment has been minimized.

Copy link
@ant0ine

ant0ine Dec 6, 2015

Author Owner
)
}
}

func ContentEncodingIsGzip(t *testing.T, r *httptest.ResponseRecorder) {
Expand Down Expand Up @@ -103,7 +120,7 @@ func (rd *Recorded) HeaderIs(headerKey, expectedValue string) {
}

func (rd *Recorded) ContentTypeIsJson() {
rd.HeaderIs("Content-Type", "application/json")
ContentTypeIsJson(rd.T, rd.Recorder)
}

func (rd *Recorded) ContentEncodingIsGzip() {
Expand Down

0 comments on commit 3152f30

Please sign in to comment.