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]