From 77dcb20a7485074d37340985d53103f94538abe6 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 26 May 2026 01:05:07 -0700 Subject: [PATCH] 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. --- seaweed-volume/src/server/handlers.rs | 101 +++++------------- .../http/production_features_test.go | 32 +++--- weed/server/common.go | 30 +----- weed/server/common_test.go | 32 ++++++ 4 files changed, 79 insertions(+), 116 deletions(-) diff --git a/seaweed-volume/src/server/handlers.rs b/seaweed-volume/src/server/handlers.rs index 633df25bf..3837b8190 100644 --- a/seaweed-volume/src/server/handlers.rs +++ b/seaweed-volume/src/server/handlers.rs @@ -838,8 +838,6 @@ pub struct ReadQueryParams { pub response_content_disposition: Option, /// Pretty print JSON response pub pretty: Option, - /// JSONP callback function name - pub callback: Option, } // ============================================================================ @@ -3402,10 +3400,6 @@ fn json_response_with_params( 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( 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=` for pretty-printed JSON and `?callback=fn` for JSONP, -/// matching Go's writeJsonError behavior. +/// Return a JSON error response, honoring `?pretty=` for pretty-printed JSON. pub(super) fn json_error_with_query( status: StatusCode, msg: impl Into, @@ -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=` from a raw query string. fn json_result_with_query(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(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. diff --git a/test/volume_server/http/production_features_test.go b/test/volume_server/http/production_features_test.go index b91ee37ef..5379463ae 100644 --- a/test/volume_server/http/production_features_test.go +++ b/test/volume_server/http/production_features_test.go @@ -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) } } diff --git a/weed/server/common.go b/weed/server/common.go index 65a09c280..990ad6398 100644 --- a/weed/server/common.go +++ b/weed/server/common.go @@ -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 } diff --git a/weed/server/common_test.go b/weed/server/common_test.go index 2e6c70bfe..18d4f1135 100644 --- a/weed/server/common_test.go +++ b/weed/server/common_test.go @@ -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", ""} + 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"}`) + } + }) + } +}