UnsupportedOperationException after Netty 4.1.60 update #7953
Assignees
Milestone
Comments
Scottmitch
changed the title
|
@ejona86 we discussed this today and IIRC we have a plan to fix this ASAP, right? @Scottmitch you have the workaround of not upgrading the netty dependency to |
|
Yeah, most users are using grpc-netty-shaded and so won't have this trouble. But we do want to help those using grpc-netty to allow them to upgrade Netty. We should implement the missing method and backport it to a 1.36.1 and 1.35.1. |
Scottmitch
added a commit
to Scottmitch/servicetalk-1
that referenced
this issue
Mar 10, 2021
Modifications: - Add workaround to ProtocolCompatibilityTest for grpc/grpc-java#7953. - Remove ServiceTalk content-length validation which is now implemented by Netty.
Scottmitch
added a commit
to Scottmitch/servicetalk-1
that referenced
this issue
Mar 10, 2021
Modifications: - Add workaround to ProtocolCompatibilityTest for grpc/grpc-java#7953.
Scottmitch
added a commit
to apple/servicetalk
that referenced
this issue
Mar 10, 2021
Modifications: - Add workaround to ProtocolCompatibilityTest for grpc/grpc-java#7953.
ejona86
added a commit
to ejona86/grpc-java
that referenced
this issue
Mar 12, 2021
Starting in Netty 4.1.60, Netty will validate Content-Length headers using getAll() and setLong(). While getAll() was documented as only used in tests, it doesn't appear it was currently used in any tests. While Http2NettyTest.contentLengthPermitted() was added to confirm that Content-Length works, it won't actually exercise any interesting behavior until we upgrade to Netty 4.1.60. However, I did test with Netty 4.1.60 and it reproduced the failure in grpc#7953 and passed with this change. Since Netty is now observing/modifying the headers, it would seem appropriate to implement a substantial portion of the Http2Headers API. However, the surface is much larger than we'd want to implement for a 'quick fix' that could be backported. In addition, it seems much of the API is just convenience methods, so it is probably appropriate to split out a AbstractHeaders class from DefaultHeaders in Netty that doesn't make any assumptions about the header storage mechanism.
ejona86
added a commit
to ejona86/grpc-java
that referenced
this issue
Mar 12, 2021
Starting in Netty 4.1.60, Netty will validate Content-Length headers using getAll() and setLong(). While getAll() was documented as only used in tests, it doesn't appear it was currently used in any tests. While Http2NettyTest.contentLengthPermitted() was added to confirm that Content-Length works, it won't actually exercise any interesting behavior until we upgrade to Netty 4.1.60. However, I did test with Netty 4.1.60 and it reproduced the failure in grpc#7953 and passed with this change. Since Netty is now observing/modifying the headers, it would seem appropriate to implement a substantial portion of the Http2Headers API. However, the surface is much larger than we'd want to implement for a 'quick fix' that could be backported. In addition, it seems much of the API is just convenience methods, so it is probably appropriate to split out a AbstractHeaders class from DefaultHeaders in Netty that doesn't make any assumptions about the header storage mechanism.
ejona86
added a commit
to ejona86/grpc-java
that referenced
this issue
Mar 12, 2021
Starting in Netty 4.1.60, Netty will validate Content-Length headers using getAll() and setLong(). While getAll() was documented as only used in tests, it doesn't appear it was currently used in any tests. While Http2NettyTest.contentLengthPermitted() was added to confirm that Content-Length works, it won't actually exercise any interesting behavior until we upgrade to Netty 4.1.60. However, I did test with Netty 4.1.60 and it reproduced the failure in grpc#7953 and passed with this change. Since Netty is now observing/modifying the headers, it would seem appropriate to implement a substantial portion of the Http2Headers API. However, the surface is much larger than we'd want to implement for a 'quick fix' that could be backported. In addition, it seems much of the API is just convenience methods, so it is probably appropriate to split out a AbstractHeaders class from DefaultHeaders in Netty that doesn't make any assumptions about the header storage mechanism.
ejona86
added a commit
that referenced
this issue
Mar 16, 2021
Starting in Netty 4.1.60, Netty will validate Content-Length headers using getAll() and setLong(). While getAll() was documented as only used in tests, it doesn't appear it was currently used in any tests. While Http2NettyTest.contentLengthPermitted() was added to confirm that Content-Length works, it won't actually exercise any interesting behavior until we upgrade to Netty 4.1.60. However, I did test with Netty 4.1.60 and it reproduced the failure in #7953 and passed with this change. Since Netty is now observing/modifying the headers, it would seem appropriate to implement a substantial portion of the Http2Headers API. However, the surface is much larger than we'd want to implement for a 'quick fix' that could be backported. In addition, it seems much of the API is just convenience methods, so it is probably appropriate to split out a AbstractHeaders class from DefaultHeaders in Netty that doesn't make any assumptions about the header storage mechanism.
ejona86
added a commit
to ejona86/grpc-java
that referenced
this issue
Mar 18, 2021
Starting in Netty 4.1.60, Netty will validate Content-Length headers using getAll() and setLong(). While getAll() was documented as only used in tests, it doesn't appear it was currently used in any tests. While Http2NettyTest.contentLengthPermitted() was added to confirm that Content-Length works, it won't actually exercise any interesting behavior until we upgrade to Netty 4.1.60. However, I did test with Netty 4.1.60 and it reproduced the failure in grpc#7953 and passed with this change. Since Netty is now observing/modifying the headers, it would seem appropriate to implement a substantial portion of the Http2Headers API. However, the surface is much larger than we'd want to implement for a 'quick fix' that could be backported. In addition, it seems much of the API is just convenience methods, so it is probably appropriate to split out a AbstractHeaders class from DefaultHeaders in Netty that doesn't make any assumptions about the header storage mechanism.
ejona86
added a commit
to ejona86/grpc-java
that referenced
this issue
Mar 18, 2021
Starting in Netty 4.1.60, Netty will validate Content-Length headers using getAll() and setLong(). While getAll() was documented as only used in tests, it doesn't appear it was currently used in any tests. While Http2NettyTest.contentLengthPermitted() was added to confirm that Content-Length works, it won't actually exercise any interesting behavior until we upgrade to Netty 4.1.60. However, I did test with Netty 4.1.60 and it reproduced the failure in grpc#7953 and passed with this change. Since Netty is now observing/modifying the headers, it would seem appropriate to implement a substantial portion of the Http2Headers API. However, the surface is much larger than we'd want to implement for a 'quick fix' that could be backported. In addition, it seems much of the API is just convenience methods, so it is probably appropriate to split out a AbstractHeaders class from DefaultHeaders in Netty that doesn't make any assumptions about the header storage mechanism.
ejona86
added a commit
that referenced
this issue
Mar 18, 2021
Starting in Netty 4.1.60, Netty will validate Content-Length headers using getAll() and setLong(). While getAll() was documented as only used in tests, it doesn't appear it was currently used in any tests. While Http2NettyTest.contentLengthPermitted() was added to confirm that Content-Length works, it won't actually exercise any interesting behavior until we upgrade to Netty 4.1.60. However, I did test with Netty 4.1.60 and it reproduced the failure in #7953 and passed with this change. Since Netty is now observing/modifying the headers, it would seem appropriate to implement a substantial portion of the Http2Headers API. However, the surface is much larger than we'd want to implement for a 'quick fix' that could be backported. In addition, it seems much of the API is just convenience methods, so it is probably appropriate to split out a AbstractHeaders class from DefaultHeaders in Netty that doesn't make any assumptions about the header storage mechanism.
ejona86
added a commit
that referenced
this issue
Mar 18, 2021
Starting in Netty 4.1.60, Netty will validate Content-Length headers using getAll() and setLong(). While getAll() was documented as only used in tests, it doesn't appear it was currently used in any tests. While Http2NettyTest.contentLengthPermitted() was added to confirm that Content-Length works, it won't actually exercise any interesting behavior until we upgrade to Netty 4.1.60. However, I did test with Netty 4.1.60 and it reproduced the failure in #7953 and passed with this change. Since Netty is now observing/modifying the headers, it would seem appropriate to implement a substantial portion of the Http2Headers API. However, the surface is much larger than we'd want to implement for a 'quick fix' that could be backported. In addition, it seems much of the API is just convenience methods, so it is probably appropriate to split out a AbstractHeaders class from DefaultHeaders in Netty that doesn't make any assumptions about the header storage mechanism.
|
There is a critical CVE for netty 4.1.52. Is there any plan to upgrade the shaded version? |
No, not for the CVEs as they do not impact gRPC. If you are using grpc-netty-shaded there is no issue. Also, netty/netty-jni-util#5 prevents us from upgrading to recent releases. (But like I said before, we do want to be compatible with new releases for grpc-netty.) |
|
Sorry to bug but is there an ETA for 1.36.x release with this fix in? |
|
Fixed by #7967. v1.35.1 and v1.36.1 are now released that include this backport. v1.36.1 beat v1.35.1 by a few hours, so don't be surprised if you don't see v1.35.1 yet. |
Scottmitch
added a commit
to Scottmitch/servicetalk-1
that referenced
this issue
Mar 25, 2021
Modifications: - Remove workarounds in ProtocolCompatibilityTest for grpc/grpc-java#7953
Scottmitch
added a commit
to apple/servicetalk
that referenced
this issue
Mar 25, 2021
Modifications: - Remove workarounds in ProtocolCompatibilityTest for grpc/grpc-java#7953
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
What version of gRPC-Java are you using?
1.36.0
What is your environment?
macOS, jdk11
What did you expect to see?
Server can process a request which contains
content-lengthheader.What did you see instead?
A change in Netty 4.1.60 introduces usage of the
Http2Headers#setLong(..)method, but grpc-java's custom header implementations do not implement this method (e.g. throwUnsupportedOperationException).Steps to reproduce the bug
4.1.60.Finalcontent-lengthheaderThe text was updated successfully, but these errors were encountered: