Go: proposal: encoding/json: make encoders set Content-Type

[empijei]

What we do

Currently encoding a JSON to a http.ResponseWriter sets a plaintext content-type:

func handler(w http.ResponseWriter, r *http.Request) {
    e := json.NewEncoder(w)
    err := e.Encode(map[string]string{"foo": "bar"})
    if err != nil {
        panic(err)
    }
}

func main() {
    r, w := httptest.NewRequest("GET", "/", nil), httptest.NewRecorder()
    handler(w, r)
    fmt.Println(w.Header().Get("Content-Type")) // text/plain; charset=utf-8
}

playground

This happens on a response recorder the same way it happens on a standard http serving stack.

The issue is that when content-type is not explicitly set, http uses http.DetectContentType on the first slice passed to Write, which will sniff as plaintext since there is no good way to sniff JSON.

What we could do

I propose to make (*json.Encoder).Encode() calls check if the passed argument has a Header() http.Header method, and if so set the appropriate content-type.

Cons:

1. encoding/json will depend on net/http, which seems less than desirable
2. It will make encoding slightly slower as it adds a type inference call to Encode calls

Pros:

1. Setting the appropriate content-type is more correct (while technically JSON is also plain text, "application/json" would be more appropriate)
2. Browsers implement some security features to protect sensitive data from being leaked cross-site, and those features heavily rely on content-type (e.g. CORB)

/cc @mvdan @rsc @dsnet @bradfitz as per owners.

[mvdan]

there is no good way to sniff JSON.

Could you expand a bit on this, for the sake of clarity?

encoding/json will depend on net/http, which seems less than desirable

I share this concern; teaching encoding/json about net/http sounds wrong :)

Also, have you thought about fixing this via static analysis? It would probably not meet the inclusion bar for vet, since I doubt this problem is common and important enough, but it could perhaps be added to staticcheck. With its fairly advanced code analysis, it should be possible to detect http handler funcs which write JSON but never set a content-type, even if there are func calls in between. CC @dominikh in case he's interested in new security checks.

[ainar-g]

Related: #10630.

[empijei]

Could you expand a bit on this, for the sake of clarity?

Sniffing is cropped at 512 bytes for performance reasons, and you cannot know if a JSON is a JSON until you have tokenized the entire thing.

I share this concern; teaching encoding/json about net/http sounds wrong :)

Yup, this is my biggest concern TBH.

Also, have you thought about fixing this via static analysis?

It seems a mess to check if one of the handlers in a chain of handlers and decorators set the CT at one point. The whole decoration chain could be set up at runtime so I think it might even be impossible.

[thoeni]

What we could do

I propose to make (*json.Encoder).Encode() calls check if the passed argument has a Header() http.Header method, and if so set the appropriate content-type.

Instead of using the Encoder we could have the json.NewEncoder check:

if _, ok := w.(http.ResponseWriter); ok {
   // Set something on the encoder so it knows what to do.
}

This would avoid possibly the cons number 2. as it won't try to figure out at each Encode() call, but number 1. would still hold true and json would know about http, which I agree on your point 1.

Cons:

1. encoding/json will depend on net/http, which seems less than desirable
2. It will make encoding slightly slower as it adds a type inference call to Encode calls

It would feel somewhat more natural to have a http.NewJSONEncoder that wraps json.NewEncoder and knows what to do with http headers when dealing with JSON, but I guess there you have no guarantee that people would use it: is this why this option might now be a good one?

[rsc]

JSON comes from many places; tying this to the encoder seems like a mistake.
Also lots of non-JSON is valid JSON, such as the one-word English text true.

Can you say more about the Pros? I am not too worried about "technically more correct".
What are the security features that are defeated by sending JSON as text/plain?

[empijei]

The more I think about this the more cons I see.

The security feature I was referring to is Cross-Origin-Read-Blocking, but even if the specification doesn't say so, browsers still seem to protect text/plain if it sniffs as JSON (I had to do some experimentation).

Since I am also not really interested in the correctness of the CT and the security benefit seems to be none, I am closing this one.

Thanks everyone for the inputs.