writeJson: drop unused JSONP branch (#9686)

* writeJson: drop unused JSONP branch

No in-tree caller uses ?callback=. Always serve application/json
with X-Content-Type-Options: nosniff.

* seaweed-volume: drop unused JSONP branch

Mirror Go: always serve application/json with
X-Content-Type-Options: nosniff.

* writeJson: drop unreachable StatusNotModified check

bodyAllowedForStatus already returns early for 304.

* test/volume_server: rename and rewrite JSONP test to assert callback is ignored

CI: /status?callback=myFunc now returns plain application/json
with X-Content-Type-Options: nosniff.
This commit is contained in:
Chris Lu
2026-05-26 01:05:07 -07:00
committed by GitHub
parent dd1b428789
commit 77dcb20a74
4 changed files with 79 additions and 116 deletions
+27 -74
View File
@@ -838,8 +838,6 @@ pub struct ReadQueryParams {
pub response_content_disposition: Option<String>,
/// Pretty print JSON response
pub pretty: Option<String>,
/// JSONP callback function name
pub callback: Option<String>,
}
// ============================================================================
@@ -3402,10 +3400,6 @@ fn json_response_with_params<T: Serialize>(
let is_pretty = params
.and_then(|params| params.pretty.as_ref())
.is_some_and(|value| !value.is_empty());
let callback = params
.and_then(|params| params.callback.as_ref())
.filter(|value| !value.is_empty())
.cloned();
let json_body = if is_pretty {
to_pretty_json(body)
@@ -3413,24 +3407,15 @@ fn json_response_with_params<T: Serialize>(
serde_json::to_string(body).unwrap()
};
if let Some(callback) = callback {
Response::builder()
.status(status)
.header(header::CONTENT_TYPE, "application/javascript")
.body(Body::from(format!("{}({})", callback, json_body)))
.unwrap()
} else {
Response::builder()
.status(status)
.header(header::CONTENT_TYPE, "application/json")
.body(Body::from(json_body))
.unwrap()
}
Response::builder()
.status(status)
.header(header::CONTENT_TYPE, "application/json")
.header("X-Content-Type-Options", "nosniff")
.body(Body::from(json_body))
.unwrap()
}
/// Return a JSON error response with optional query string for pretty/JSONP support.
/// Supports `?pretty=<any non-empty value>` for pretty-printed JSON and `?callback=fn` for JSONP,
/// matching Go's writeJsonError behavior.
/// Return a JSON error response, honoring `?pretty=<any non-empty value>` for pretty-printed JSON.
pub(super) fn json_error_with_query(
status: StatusCode,
msg: impl Into<String>,
@@ -3438,18 +3423,10 @@ pub(super) fn json_error_with_query(
) -> Response {
let body = serde_json::json!({"error": msg.into()});
let (is_pretty, callback) = if let Some(q) = query {
let pretty = q
.split('&')
.any(|p| p.starts_with("pretty=") && p.len() > "pretty=".len());
let cb = q
.split('&')
.find_map(|p| p.strip_prefix("callback="))
.map(|s| s.to_string());
(pretty, cb)
} else {
(false, None)
};
let is_pretty = query.is_some_and(|q| {
q.split('&')
.any(|p| p.starts_with("pretty=") && p.len() > "pretty=".len())
});
let json_body = if is_pretty {
to_pretty_json(&body)
@@ -3457,35 +3434,19 @@ pub(super) fn json_error_with_query(
serde_json::to_string(&body).unwrap()
};
if let Some(cb) = callback {
let jsonp = format!("{}({})", cb, json_body);
Response::builder()
.status(status)
.header(header::CONTENT_TYPE, "application/javascript")
.body(Body::from(jsonp))
.unwrap()
} else {
Response::builder()
.status(status)
.header(header::CONTENT_TYPE, "application/json")
.body(Body::from(json_body))
.unwrap()
}
Response::builder()
.status(status)
.header(header::CONTENT_TYPE, "application/json")
.header("X-Content-Type-Options", "nosniff")
.body(Body::from(json_body))
.unwrap()
}
/// Return a JSON response with optional pretty/JSONP support from raw query string.
/// Matches Go's writeJsonQuiet behavior for write success responses.
/// Return a JSON response honoring `?pretty=<any non-empty value>` from a raw query string.
fn json_result_with_query<T: Serialize>(status: StatusCode, body: &T, query: &str) -> Response {
let (is_pretty, callback) = {
let pretty = query
.split('&')
.any(|p| p.starts_with("pretty=") && p.len() > "pretty=".len());
let cb = query
.split('&')
.find_map(|p| p.strip_prefix("callback="))
.map(|s| s.to_string());
(pretty, cb)
};
let is_pretty = query
.split('&')
.any(|p| p.starts_with("pretty=") && p.len() > "pretty=".len());
let json_body = if is_pretty {
to_pretty_json(body)
@@ -3493,20 +3454,12 @@ fn json_result_with_query<T: Serialize>(status: StatusCode, body: &T, query: &st
serde_json::to_string(body).unwrap()
};
if let Some(cb) = callback {
let jsonp = format!("{}({})", cb, json_body);
Response::builder()
.status(status)
.header(header::CONTENT_TYPE, "application/javascript")
.body(Body::from(jsonp))
.unwrap()
} else {
Response::builder()
.status(status)
.header(header::CONTENT_TYPE, "application/json")
.body(Body::from(json_body))
.unwrap()
}
Response::builder()
.status(status)
.header(header::CONTENT_TYPE, "application/json")
.header("X-Content-Type-Options", "nosniff")
.body(Body::from(json_body))
.unwrap()
}
/// Extract JWT token from query param, Authorization header, or Cookie.
@@ -75,7 +75,7 @@ func TestStatsEndpoints(t *testing.T) {
}
}
func TestStatusPrettyJsonAndJsonp(t *testing.T) {
func TestStatusPrettyJsonAndCallbackIgnored(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
@@ -93,29 +93,29 @@ func TestStatusPrettyJsonAndJsonp(t *testing.T) {
if len(lines) < 3 {
t.Fatalf("/status?pretty=y expected multi-line indented JSON, got %d lines: %s", len(lines), string(prettyBody))
}
// Verify the body is valid JSON
var prettyPayload map[string]interface{}
if err := json.Unmarshal(prettyBody, &prettyPayload); err != nil {
t.Fatalf("/status?pretty=y is not valid JSON: %v", err)
}
// ?callback=myFunc — expect JSONP wrapping
jsonpResp := framework.DoRequest(t, client, mustNewRequest(t, http.MethodGet, cluster.VolumeAdminURL()+"/status?callback=myFunc"))
jsonpBody := framework.ReadAllAndClose(t, jsonpResp)
if jsonpResp.StatusCode != http.StatusOK {
t.Fatalf("/status?callback=myFunc expected 200, got %d", jsonpResp.StatusCode)
// ?callback=myFunc — must be ignored; response is plain JSON with nosniff.
cbResp := framework.DoRequest(t, client, mustNewRequest(t, http.MethodGet, cluster.VolumeAdminURL()+"/status?callback=myFunc"))
cbBody := framework.ReadAllAndClose(t, cbResp)
if cbResp.StatusCode != http.StatusOK {
t.Fatalf("/status?callback=myFunc expected 200, got %d", cbResp.StatusCode)
}
bodyStr := string(jsonpBody)
if !strings.HasPrefix(bodyStr, "myFunc(") {
t.Fatalf("/status?callback=myFunc expected body to start with 'myFunc(', got prefix: %q", bodyStr[:min(len(bodyStr), 30)])
if ct := cbResp.Header.Get("Content-Type"); !strings.Contains(ct, "application/json") {
t.Fatalf("/status?callback=myFunc expected Content-Type application/json, got %q", ct)
}
trimmed := strings.TrimRight(bodyStr, "\n; ")
if !strings.HasSuffix(trimmed, ")") {
t.Fatalf("/status?callback=myFunc expected body to end with ')', got suffix: %q", trimmed[max(0, len(trimmed)-10):])
if nosniff := cbResp.Header.Get("X-Content-Type-Options"); nosniff != "nosniff" {
t.Fatalf("/status?callback=myFunc expected X-Content-Type-Options nosniff, got %q", nosniff)
}
// Content-Type should be application/javascript for JSONP
if ct := jsonpResp.Header.Get("Content-Type"); !strings.Contains(ct, "javascript") {
t.Fatalf("/status?callback=myFunc expected Content-Type containing 'javascript', got %q", ct)
if strings.Contains(string(cbBody), "myFunc(") {
t.Fatalf("/status?callback=myFunc must not wrap response in callback; body: %q", string(cbBody))
}
var cbPayload map[string]interface{}
if err := json.Unmarshal(cbBody, &cbPayload); err != nil {
t.Fatalf("/status?callback=myFunc is not valid JSON: %v", err)
}
}
+4 -26
View File
@@ -110,32 +110,10 @@ func writeJson(w http.ResponseWriter, r *http.Request, httpStatus int, obj inter
r.Method, r.URL.String(), httpStatus, string(bytes))
}
callback := r.FormValue("callback")
if callback == "" {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(httpStatus)
if httpStatus == http.StatusNotModified {
return
}
_, err = w.Write(bytes)
} else {
w.Header().Set("Content-Type", "application/javascript")
w.WriteHeader(httpStatus)
if httpStatus == http.StatusNotModified {
return
}
if _, err = w.Write([]uint8(callback)); err != nil {
return
}
if _, err = w.Write([]uint8("(")); err != nil {
return
}
fmt.Fprint(w, string(bytes))
if _, err = w.Write([]uint8(")")); err != nil {
return
}
}
w.Header().Set("Content-Type", "application/json")
w.Header().Set("X-Content-Type-Options", "nosniff")
w.WriteHeader(httpStatus)
_, err = w.Write(bytes)
return
}
+32
View File
@@ -1,6 +1,8 @@
package weed_server
import (
"net/http"
"net/http/httptest"
"strings"
"testing"
)
@@ -29,3 +31,33 @@ func TestParseURL(t *testing.T) {
}
}
}
func TestWriteJsonNoJSONP(t *testing.T) {
// callback= must be ignored; response is always application/json with nosniff.
cases := []string{"", "myCb", "<script>alert(1)</script>"}
for _, cb := range cases {
t.Run("callback="+cb, func(t *testing.T) {
url := "/x"
if cb != "" {
url += "?callback=" + cb
}
r := httptest.NewRequest(http.MethodGet, url, nil)
w := httptest.NewRecorder()
if err := writeJson(w, r, http.StatusOK, map[string]string{"k": "v"}); err != nil {
t.Fatalf("writeJson: %v", err)
}
if w.Code != http.StatusOK {
t.Errorf("status: got %d want 200", w.Code)
}
if got := w.Header().Get("Content-Type"); got != "application/json" {
t.Errorf("Content-Type: got %q want application/json", got)
}
if got := w.Header().Get("X-Content-Type-Options"); got != "nosniff" {
t.Errorf("X-Content-Type-Options: got %q want nosniff", got)
}
if got := w.Body.String(); got != `{"k":"v"}` {
t.Errorf("body: got %q want %q", got, `{"k":"v"}`)
}
})
}
}