Skip to content

Commit 1379030

Browse files
authored
fix: /response-headers does not need escaping by default (#208)
The fix in commit 0decfd1 for a potential XSS vulnerability[1] in the `/response-headers` endpoint made an unintentionally breaking change, by HTML-escaping the body of the response when no explicit `Content-Type` is specified in the incoming request. We do not need to escape by default, because we default to a safe JSON content type in that case. [1]: GHSA-528q-4pgm-wvg2
1 parent 0decfd1 commit 1379030

File tree

2 files changed

+21
-7
lines changed

2 files changed

+21
-7
lines changed

httpbin/handlers.go

+10-6
Original file line numberDiff line numberDiff line change
@@ -341,19 +341,23 @@ func (h *HTTPBin) Unstable(w http.ResponseWriter, r *http.Request) {
341341
// values in the JSON response body will be escaped.
342342
func (h *HTTPBin) ResponseHeaders(w http.ResponseWriter, r *http.Request) {
343343
args := r.URL.Query()
344+
345+
// only set our own content type if one was not already set based on
346+
// incoming request params
344347
contentType := args.Get("Content-Type")
348+
if contentType == "" {
349+
contentType = jsonContentType
350+
args.Set("Content-Type", contentType)
351+
}
345352

346-
// response headers are not escaped, regardless of content type
353+
// actual HTTP response headers are not escaped, regardless of content type
354+
// (unlike the JSON serialized representation of those headers in the
355+
// response body, which MAY be escaped based on content type)
347356
for k, vs := range args {
348357
for _, v := range vs {
349358
w.Header().Add(k, v)
350359
}
351360
}
352-
// only set our own content type if one was not already set based on
353-
// incoming request params
354-
if contentType == "" {
355-
w.Header().Set("Content-Type", jsonContentType)
356-
}
357361

358362
// if response content type is dangrous, escape keys and values before
359363
// serializing response body

httpbin/handlers_test.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -1241,6 +1241,13 @@ func TestResponseHeaders(t *testing.T) {
12411241
resultValues := result[k]
12421242
assert.DeepEqual(t, resultValues, expectedValues, "JSON response headers mismatch")
12431243
}
1244+
1245+
// if no content-type is specified in the request params, the response
1246+
// defaults to JSON.
1247+
//
1248+
// Note that if this changes, we need to ensure we maintain safety
1249+
// around escapig HTML in the response (see the subtest below)
1250+
assert.Header(t, resp, "Content-Type", jsonContentType)
12441251
})
12451252

12461253
t.Run("override content-type", func(t *testing.T) {
@@ -1271,8 +1278,11 @@ func TestResponseHeaders(t *testing.T) {
12711278
{"text/plain", false},
12721279
{"application/octet-string", false},
12731280

1281+
// if no content-type is provided, we default to JSON, which is
1282+
// safe
1283+
{"", false},
1284+
12741285
// everything else requires escaping
1275-
{"", true},
12761286
{"application/xml", true},
12771287
{"image/png", true},
12781288
{"text/html; charset=utf8", true},

0 commit comments

Comments
 (0)