-
Task
-
Resolution: Fixed
-
Major
-
None
On the prototype branch of Jenkins core, Security3030Test fails with SocketException on Jetty 12. I have temporarily worked around the issue with the following hack:
diff --git a/test/src/test/java/jenkins/security/Security3030Test.java b/test/src/test/java/jenkins/security/Security3030Test.java index e2d1e4bf5b..5a5ef07f85 100644 --- a/test/src/test/java/jenkins/security/Security3030Test.java +++ b/test/src/test/java/jenkins/security/Security3030Test.java @@ -39,6 +39,7 @@ import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.lang.reflect.Field; +import java.net.SocketException; import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Random; @@ -212,7 +213,11 @@ public class Security3030Test { ByteArrayOutputStream baos = new ByteArrayOutputStream(); writeMultipartFormDataBody(baos, boundary, files, other, fileSize); request.setRequestBody(baos.toString()); - wc.getPage(request); + try { + wc.getPage(request); + } catch (SocketException e) { + // TODO Why is this behavior changing? + } return endpoint.getActual(); }
The root cause of this change in behavior should be understood. It may actually be a desirable Jetty security enhancement. If the behavior is expected, the test should be adapted (at the very least, to assert that this now expected exception actually occurs). If the behavior is unexpected, the bug should be fixed and the test change reverted. Before submitting the PR to the prototype branch to fix this ticket, ensure that mvn clean verify -Dtest=jenkins.security.Security3030Test passes on both Linux and Windows.
- is blocking
-
JENKINS-73130 Upgrade core from Jetty 10.x to 12.x (EE 8)
-
- Closed
-
I debugged this today and found that on Jetty 12 the server is correctly rejecting the request as before with the correct exception (no regression) but is now rejecting it so quickly that the server sends the rejection (or in the test's case, asserts the expected exception and then sends a success message, rather than sending the rejection as would be the case in production) before the client has finished sending the request. So the server sends the response and closes the socket, while the client still has the socket open and is still sending out the rest of the request. The client receives a broken pipe error because the server has closed the socket. This didn't happen in Jetty 10 presumably because the server was slower and the client would finish sending the request before the server had decided to reject it.
I was able to get the test to pass in Jetty 12 easily enough:
The only remaining question in my mind is the following: is it OK that we're now rejecting these requests so quickly that clients might not even be done sending the request at the time that we reject the request and send the error back to the client? I personally think it is, and that the client should deal with this gracefully, something that HtmlUnit/Apache HttpClient 4.x (the client in the test mentioned above) isn't doing. The alternative, if we want to give this type of client the exact same behavior as before, would be to drain the read end of the socket in org.kohsuke.stapler.RequestImpl#parseMultipartFormData before throwing the ServletException, but to me this is a bad idea for a few reasons:
So I think adapting the test as described above is the best solution. danielbeck do you have any opinion here?