Skip to content

Use Http2Headers.size() instead of isEmpty()#13717

Merged
idelpivnitskiy merged 1 commit intonetty:4.1from
idelpivnitskiy:size
Dec 5, 2023
Merged

Use Http2Headers.size() instead of isEmpty()#13717
idelpivnitskiy merged 1 commit intonetty:4.1from
idelpivnitskiy:size

Conversation

@idelpivnitskiy
Copy link
Member

Motivation:

grpc-java prior to version 1.59.1 does not implement Http2Headers.isEmpty() method (it throws
UnsupportedOperationException). While the fix was released in 1.59.1, there is another issue with circular dependencies between grpc-core and grpc-util modules. To unblock Netty users that also depend on grpc-java, we can check the size.

Modifications:

  • Use Http2Headers.size() instead of Http2Headers.isEmpty() in DefaultHttp2ConnectionDecoder;

Result:

Users of older grpc-java versions can independently bump Netty version.

Motivation:

grpc-java prior to version 1.59.1 does not implement
`Http2Headers.isEmpty()` method (it throws
`UnsupportedOperationException`). While the fix was released
in 1.59.1, there is another issue with circular dependencies
between `grpc-core` and `grpc-util` modules. To unblock Netty
users that also depend on grpc-java, we can check the size.

Modifications:

- Use `Http2Headers.size()` instead of `Http2Headers.isEmpty()`
in `DefaultHttp2ConnectionDecoder`;

Result:

Users of older grpc-java versions can independently bump Netty
version.
@normanmaurer
Copy link
Member

Not ideal but I guess the best we can do

@idelpivnitskiy idelpivnitskiy merged commit 8b20d6b into netty:4.1 Dec 5, 2023
@idelpivnitskiy idelpivnitskiy deleted the size branch December 5, 2023 21:13
@normanmaurer normanmaurer added this to the 4.1.102.Final milestone Dec 6, 2023
@ejona86
Copy link
Member

ejona86 commented Dec 13, 2023

Does that mean y'all are getting bit from the Gradle side of things (grpc/grpc-java#10701)? (The other case we are aware of is Bazel, grpc/grpc-java#10576 .) The oss-licenses-plugin was related to Android, so is there another common plugin that is impacted by gradle/gradle#22850?

@normanmaurer
Copy link
Member

@idelpivnitskiy is the right person to answer this question :) Can you add some details ?

@idelpivnitskiy
Copy link
Member Author

idelpivnitskiy commented Dec 13, 2023

Personally, I didn't try it myself, but we received concerns/hesitations from multiple projects, including gradle and bazel based.

@ejona86
Copy link
Member

ejona86 commented Dec 13, 2023

Thanks. Okay. So this was a "let's not risk stuff when doing a security update." That's fair.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants