LuciferYang commented on code in PR #57138:
URL: https://github.com/apache/spark/pull/57138#discussion_r3549243347


##########
core/src/test/scala/org/apache/spark/deploy/rest/StandaloneRestSubmitSuite.scala:
##########
@@ -599,6 +599,24 @@ class StandaloneRestSubmitSuite extends SparkFunSuite {
     }
   }
 
+  test("SPARK-58049: Reject an over-sized request body with 
SC_REQUEST_ENTITY_TOO_LARGE") {
+    val conf = new SparkConf()
+    conf.set(MASTER_REST_SERVER_MAX_REQUEST_BODY_SIZE.key, "1k")
+    val localhost = Utils.localHostName()
+    val securityManager = new SecurityManager(conf)
+    rpcEnv =
+      Some(RpcEnv.create("rest-with-maxRequestBodySize", localhost, 0, conf, 
securityManager))
+    val fakeMasterRef = rpcEnv.get.setupEndpoint("fake-master", new 
DummyMaster(rpcEnv.get))
+    val _server = new StandaloneRestServer(localhost, 0, conf, fakeMasterRef, 
"spark://fake:7077")
+    server = Some(_server)
+    val port = _server.start()
+    val v = RestSubmissionServer.PROTOCOL_VERSION
+    val path = s"http://$localhost:$port/$v/submissions/create";
+    val (response, code) = sendHttpRequestWithResponse(path, "POST", "x" * 
4096)

Review Comment:
   The new test writes the body through a buffered `HttpURLConnection`, so it 
sends an accurate `Content-Length` and the server rejects at the 
`getContentLengthLong > limit` early branch. The defense this PR actually adds 
is the `readNBytes(limit + 1)` + `bytes.length > limit` fallback that guards 
against a missing or spoofed `Content-Length`, and that path has no test 
coverage right now; the accept boundary (a body exactly at `limit`) is untested 
too. Could we add two cases: one using `conn.setChunkedStreamingMode(0)` to 
send an over-limit body (so `Content-Length` is -1 and the `readNBytes` branch 
is forced), asserting 413; and one for the accept boundary, being careful not 
to use `"x" * limit` (invalid JSON, rejected as 400 after the size check) but 
instead `constructSubmitRequest(...)` to build a valid request and set the 
limit to its exact byte length, asserting `SC_OK`. That keeps this security 
path from silently regressing later.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to