fix: harden gRPC HTTP/2 stream limits - #18
Conversation
| <artifact name="guava-33.6.0-jre.jar"> | ||
| <sha256 value="dc573e1fca4fd5454f4a5fd3d7da2df03002876a4175bafc14a95980dd7713b3" origin="Generated by Gradle"/> | ||
| </artifact> | ||
| <artifact name="guava-33.6.0-jre.module"> |
There was a problem hiding this comment.
please add guava:33.6.0-jre.pom sha256
b4f11ec to
3fc7b23
Compare
|
|
||
| @Test | ||
| public void shouldRejectNonPositiveStreamLimit() { | ||
| assertThrows(IllegalArgumentException.class, |
There was a problem hiding this comment.
IllegalArgumentException exception = assertThrows(
IllegalArgumentException.class,
() -> GrpcNettyMaxConcurrentStreamsLimiter.newPlaintextNegotiator(limit));
assertEquals("maxConcurrentStreams must be positive", exception.getMessage());
| // https://github.com/grpc/grpc-java/pull/12319, Add support for macOS aarch64 with universal binary | ||
| // https://github.com/grpc/grpc-java/pull/11371 , 1.64.x is not supported CentOS 7. | ||
| ProtocGenVersion: isArm64 || isMac ? '1.81.0' : '1.60.0' | ||
| ProtocGenVersion: isArm64 || isMac ? '1.83.0' : '1.60.0' |
There was a problem hiding this comment.
gRPC version 1.83.0 is still duplicated in build.gradle and protocol/build.gradle. Define grpcVersion once in the root ext block and use it for both the runtime dependencies and ARM/macOS code generator.
| def protobufVersion = '3.25.8' | ||
| // keep same version as protoc-gen-grpc-java for arm64 or macOS, see rootProject.archInfo.requires.ProtocGenVersion | ||
| def grpcVersion = '1.81.0' | ||
| def grpcVersion = '1.83.0' |
There was a problem hiding this comment.
The Netty 4.2 allocator migration has not been addressed. The upgrade implicitly changes Netty’s default allocator from pooled to adaptive, but the PR neither pins io.netty.allocator.type=pooled nor provides representative load or soak-test results for the adaptive allocator. Netty recommends retaining the pooled allocator during the initial 4.2 rollout and evaluating adaptive allocation separately. See the Netty 4.2 Migration Guide.
There was a problem hiding this comment.
Obsolete verification entries remain, including gRPC 1.81.0 and Netty 4.1.132.Final. This does not affect the resolved runtime classpath, but it does not satisfy the earlier requirement to clean up expired verification metadata.
| <artifact name="guava-parent-33.4.0-jre.pom"> | ||
| <sha256 value="3a499ed34a0d9ee0f1bcc39230021a1cd4e2f7dd0426ab6844f585465d41dcd7" origin="Generated by Gradle"/> | ||
| </artifact> | ||
| </component> |
There was a problem hiding this comment.
gradle/verification-metadata.xml is missing guava-parent:33.6.0-jre.pom (and guava-33.6.0-jre.pom).
grpc-core/grpc-api 1.83.0 pull guava:33.6.0-android
3fc7b23 to
ab1872f
Compare
ab1872f to
e672eb5
Compare
e672eb5 to
1275864
Compare
a78bf35
into
xxo1shine:fix/discard-unknown-fields-for-p2p-messages
Summary
1.81.0to1.83.04.2.15.Finalfrom grpc-java 1.83.0 instead of overriding Netty through a separate BOM0to that secure defaultRpcServiceregression coverageRoot cause
grpc-java constructs
DefaultHttp2Connection, its encoder, and its decoder directly instead of using Netty'sHttp2ConnectionHandlerBuilder. It advertisesSETTINGS_MAX_CONCURRENT_STREAMS, but Netty normally applies that setting toconnection.remote().maxActiveStreams()only after the peer acknowledges it.Netty's fixed builder applies the limit immediately, but grpc-java bypasses that builder. java-tron therefore installs a small plaintext protocol-negotiator wrapper that applies the same validated value before delegating handler installation:
The compatibility shim is tracked by grpc-java#12930 and can be removed after grpc-java performs equivalent server-side enforcement.
Dependency selection
4.2.15.Final4.2.15.Final4.2.15.FinalThe standalone
nettyVersionandnetty-bomdeclaration have been removed. The resolved runtime classpath contains 13 Netty modules, all at4.2.15.Final, with no Netty 4.1.x or mixed 4.2.x versions.Netty 4.2 split its former codec artifact into smaller modules. java-tron's backup server directly uses the protobuf frame codec, which is not a grpc-netty transitive dependency, so
netty-codec-protobufis declared explicitly at the same4.2.15.Finalversion. Its protobuf-java and javanano transitives remain excluded.The macOS/ARM64
protoc-gen-grpc-javaversion is aligned to1.83.0, as required by the existing build configuration. grpc-java 1.83.0 resolves protobuf-java3.25.9and its corresponding dependency updates.Netty 4.2 allocator migration
Netty 4.2 changes the default allocator from
pooledtoadaptive. Following Netty's staged migration guidance, both java-tron VM option files explicitly set-Dio.netty.allocator.type=pooled. The adaptive allocator can be evaluated separately with representative load and soak testing.The gRPC version is defined once in the root Gradle extension and reused by runtime dependencies and the ARM/macOS code generator. Obsolete gRPC
1.81.0and Netty4.1.132.Finalverification entries have been removed.Configuration impact
node.rpc.maxConcurrentCallsPerConnectionnow defaults to 100.Previous releases treated an explicit value of
0as unlimited. This PR maps0to the secure default of 100 and logs a startup migration warning. Operators that require more than 100 concurrent calls on one connection must configure an explicit positive value. Negative values now fail during configuration parsing with a clear parameter error.Validation on target branch
Passed locally after applying the change to
fix/discard-unknown-fields-for-p2p-messages:4.2.15.Final-Dio.netty.allocator.type=pooled./gradlew :common:test --tests org.tron.core.config.args.NodeConfigTestRpcServiceHttp2SecurityTestKeepAliveMessageTestandMessageUnknownFieldTest./gradlew :framework:checkstyleMain :framework:checkstyleTest :framework:assemble :protocol:assembleThe branch is one squashed commit ahead of
fix/discard-unknown-fields-for-p2p-messages.