[jdk11u-dev] RFR: 8245245: WebSocket can lose the URL encoding of URI query parameters

[Michal Karm Babacek]

On Tue, 6 Dec 2022 13:47:09 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Proposes to backport [JDK-8245245](https://bugs.openjdk.org/browse/JDK-8245245).
>> 
>> The backport is clean as far as the actual `OpeningHandshake.java` goes. The test needed a little tweak so as to compile with `SimpleSSLContext` and also to handle the fact that the erroneous response does not bring a response body.
>> 
>> The test passes with the patch, fails without it.
>> 
>> 
>> $ make clean run-test TEST="jtreg:test/jdk/java/net/httpclient/websocket/HandshakeUrlEncodingTest.java"
>> ...
>> ==============================
>> Test summary
>> ==============================
>>    TEST                                              TOTAL  PASS  FAIL ERROR   
>>    jtreg:test/jdk/java/net/httpclient/websocket/HandshakeUrlEncodingTest.java
>>                                                          1     1     0     0   
>> ==============================
>> TEST SUCCESS
>> 
>> Stopping sjavac server
>> Finished building targets 'clean run-test' in configuration 'linux-x86_64-normal-server-release'
>> 
>> In addition to that, I compiled and executed the original `WebSocketTest.java` reproducer found on  [JDK-8245245](https://bugs.openjdk.org/browse/JDK-8245245) JIRA.
>> 
>> 
>> ## Unpatched Temurin-11.0.17+8  ❌ 
>> 
>> $ java WebSocketTest 
>> Http Request
>> http://localhost:8000/?raw=abc+def/ghi=xyz&encoded=abc%2Bdef%2Fghi%3Dxyz
>> Server RequestURI: /?raw=abc+def/ghi=xyz&encoded=abc%2Bdef%2Fghi%3Dxyz
>> WebSocket Request
>> ws://localhost:8000/?&raw=abc+def/ghi=xyz&encoded=abc%2Bdef%2Fghi%3Dxyz
>> Server RequestURI: /?&raw=abc+def/ghi=xyz&encoded=abc+def/ghi=xyz
>> 
>> 
>> ## Patched jdk11u ✔ 
>> 
>> $ java WebSocketTest 
>> Http Request
>> http://localhost:8000/?raw=abc+def/ghi=xyz&encoded=abc%2Bdef%2Fghi%3Dxyz
>> Server RequestURI: /?raw=abc+def/ghi=xyz&encoded=abc%2Bdef%2Fghi%3Dxyz
>> WebSocket Request
>> ws://localhost:8000/?&raw=abc+def/ghi=xyz&encoded=abc%2Bdef%2Fghi%3Dxyz
>> Server RequestURI: /?&raw=abc+def/ghi=xyz&encoded=abc%2Bdef%2Fghi%3Dxyz
>> 
>> The patched version correctly leaves the latter part of the query param encoded.
>
> It is important that the test checks the URI received by the server, and that's probably why the body was used here. The server writes the URI it receives in the response body. This provides an end-to-end check that what was received is what we expected to send. Note that the server doesn't actually supports WebSocket and that's why it always replies with 400.

Hello @dfuch, @jerboaa, @phohensee, @jaikiran 

I'd like this tets change to go forward: https://github.com/openjdk/jdk/pull/11486
It correctly fails without JDK-8245245 and passes with it. It is not necessary to be checking the body.

When that test change is integrated, I'd initiate its backport.  Close this pr (https://github.com/openjdk/jdk11u-dev/pull/1558) and open a new one, that would just cleanly add JDK-8245245...


I hope that is all right process wise.

-------------

PR: https://git.openjdk.org/jdk11u-dev/pull/1558