Bump netty version to use ALPN support needed for JDK8u282. #662
Conversation
|
@vivdesh Is there any test done with this change? If possible, you can create a hotfix version of the container and test it with some services. |
| @@ -261,7 +261,7 @@ public void testMaxHeaderSize() throws InterruptedException, IOException, Timeou | |||
| { | |||
| testHeaderSize(TEST_MAX_HEADER_SIZE - 1, RESPONSE_OK); | |||
|
|
|||
| testHeaderSize(TEST_MAX_HEADER_SIZE, RESPONSE_OK); | |||
| testHeaderSize(TEST_MAX_HEADER_SIZE, TOO_LARGE); | |||
There was a problem hiding this comment.
@vivdesh Hi, could you explain why we need to change this test?
There was a problem hiding this comment.
Mostly strict parsing of the http header causing this test to fail. netty/netty#10058.
Please let me know where we can change the code according to this?
There was a problem hiding this comment.
@vivdesh Thanks for the additional information. If the original test failed because our header generator doesn't follow strictly the HTTP RFC, we probably better fix it.
There was a problem hiding this comment.
Actually, there is already a fix from https://github.com/linkedin/rest.li/pull/595/files#diff-bd070627be0acf661a65543c18644309364d2aaf26967596cfdce517c07a6a12R232 by @tjni
Looking more, this change appears to be a subset of #595 which also introducing a newer version of Netty. It makes sense to me to review and merge that commit instead.
With JDK8u282, the ALPN APIs are merged with JDK itself and the ALPN jar is not required. The support for this is added in the netty. Without this newer netty, we get "Unable to wrap SSLEngine of type sun.security.ssl.SSLEngineImpl" runtime error. So this PR updates the netty to avoid this error.