TCK: HttpUpgradeHandler test incorrectly assumes reading of buffered POST data

[brideck]

In Open Liberty, we see an intermittent failure in https://github.com/jakartaee/servlet/blob/master/tck/tck-runtime/src/main/java/servlet/tck/api/jakarta_servlet_http/httpupgradehandler/HttpUpgradeHandlerTests.java due to a faulty assumption in the test that can cause a race between the upgrade and the sending of the chunks of data ("Hello", "World").

The test is currently implemented to send the upgrade request followed by two chunks of data. It is expected that the upgrade should occur and that both chunks of data be handled post-upgrade. However, Section 2.3.3.5 of the Servlet spec states that the servlet container shall "pass a WebConnection to allow the protocol handler access to the data streams" after completing the upgrade. This means that the TCKReadListener's onDataAvailable method is only triggered by new data entering those streams after the upgrade is complete. In our failing case, one or both chunks of data are sent and received before the upgrade has completed, so the TCKReadListener never sees them. For this to consistently pass as written, the test seems to be assuming that the TCKReadListener will be able to see the buffered data that's already been received, which is not stated as a requirement in the spec.

An easy way to make this test more reliable is to read for the successful upgrade response (EXPECTED_RESPONSE1) after making the upgrade request, but before sending the chunks of data:

Send upgrade request
Read for EXPECTED_RESPONSE1
Send "Hello"
Send "World"
Read for EXPECTED_RESPONSE2/3

[markt-asf]

TL;DR For Servlet <= 6.0, this is an Open Liberty bug that needs to be fixed, not a TCK bug. There is an option to change the Servlet specifcation for Servlet >= 6.1

The Servlet specification, correctly, makes no statement regarding the timing of sending data for the upgraded connection with respect to the HTTP 101 response.

It is RFC compliant to send data for an upgraded connection prior to receiving the HTTP 101 response. Quoting RFC 9110

A client cannot begin using an upgraded protocol on the connection until it has completely sent the request message (i.e., the client can't change the protocol it is sending in the middle of a message).

Nothing in the current RFCs require the client to wait for the upgrade to be confirmed before using the protocol.

A number of RFCs for upgraded protocols do require the client to wait for the upgrade to be confirmed but those are protocol specific requirements, not generic upgrade requirements.

There is a general concern that optimistic upgrade (sending data using the upgraded protocol before the server has confirmed the upgrade) is problematic and there is an RFC in draft that would explicitly require all future upgrade protocols to protect against the security risks associated with optimistic upgrade. However, that RFC does not modify the language in RFC 9110 and optimistic upgrade remains specification compliant.

The TCK test could be improved to send the upgrade request and the data for the upgrade protocol at the same time which would result in a consistent TCK failure for Open Liberty and any similarly implemented servers.

We could modify the Servlet specification to reject any attempt at optimistic upgrade with a 400 response. I'm reluctant to do this as there are scenarios (which are the most likely use cases for a custom HttpUpgradeHandler) where optimistic upgrade is safe and it provides a performance improvement.

[brideck]

Thanks, Mark. I've passed this back to web dev for rebuttal/acquiescence.

[markt-asf]

Given the lack of further response combined with the current behaviour being aligned with the RFCs, I am closing this one.