The KeepAliveManager clamps the keepalive and keepalive time values such that the result is larger than the minimum values specified here. Therefore, a second check here is unnecessary.
All the changes outside libs.versions.toml and examples were
because of ErrorProne. It didn't actually find anything to fix; signal
vs noise has gotten pretty bad with the newer checks.
Status was changed for ErrorProne's SuperCallToObjectMethod. With the
old code it didn't notice the trivial implementation. The fail-for-test
code wasn't used, so it was easiest to just remove it.
Some of the libs had their versions inlined; now that we have
:checkForUpdates it isn't much of a risk for versions to diverge when
there's only a few artifacts sharing a version. If we need 4+ artifacts
to have the same version, then it makes sense to still use a shared
version.
Dependencies not upgraded: google-auth-libray, mockito, netty, cronet
The behavior purposefully mirrors that of Netty's
AbstractHttp2ConnectionHandlerBuilder.decoderEnforceMaxRstFramesPerWindow().
That API is not available to our code as we extend the
Http2ConnectionHandler, but we want our API to be able to delegate to
Netty's in the future if that ever becomes possible.
Huffman in the datacenter doesn't add much value in the common cases. It could be useful to turn on huffman based on the connection latency (say, >10ms means "assume cross-datacenter") but the Netty API doesn't lend it to that. The savings here aren't huge and it is expensive; the table provides the biggest savings.
* core, netty, okhttp: implement new logic for nameResolverFactory API in channelBuilder
fix ManagedChannelImpl to use NameResolverRegistry instead of NameResolverFactory
fix the ManagedChannelImplBuilder and remove nameResolverFactory
* Integrate target parsing and NameResolverProvider searching
Actually creating the name resolver is now delayed to the end of
ManagedChannelImpl.getNameResolver; we don't want to call into the name
resolver to determine if we should use the name resolver.
Added getDefaultScheme() to NameResolverRegistry to avoid needing
NameResolver.Factory.
---------
Co-authored-by: Eric Anderson <ejona@google.com>
This breaks the ABI of the classes listed below.
Users that recompiled their code using grpc-java [`v1.36.0`]
(https://github.com/grpc/grpc-java/releases/tag/v1.36.0) (released on
Feb 23, 2021) and later, ARE NOT AFFECTED.
Users that compiled their source using grpc-java earlier than
[`v1.36.0`]
(https://github.com/grpc/grpc-java/releases/tag/v1.36.0) need to
recompile when upgrading to grpc-java `v1.59.0`. Otherwise the code
will fail on runtime with `NoSuchMethodError`. For example, code:
```java
NettyChannelBuilder.forTarget("localhost:100").maxRetryAttempts(2);
```
Will fail with
> `java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractManagedChannelImplBuilder
io.grpc.netty.NettyChannelBuilder.maxRetryAttempts(int)'`
**Affected classes**
Class `AbstractManagedChannelImplBuilder` is deleted, and no longer in
the class hierarchy of the channel builders:
- `io.grpc.netty.NettyChannelBuilder`
- `io.grpc.okhttp.OkhttpChannelBuilder`
- `grpc.cronet.CronetChannelBuilder`
This breaks the ABI of the classes listed below.
Users that recompiled their code using grpc-java [`v1.36.0`]
(https://github.com/grpc/grpc-java/releases/tag/v1.36.0) (released on
Feb 23, 2021) and later, ARE NOT AFFECTED.
Users that compiled their source using grpc-java earlier than
[`v1.36.0`]
(https://github.com/grpc/grpc-java/releases/tag/v1.36.0) need to
recompile when upgrading to grpc-java `v1.59.0`. Otherwise the code
will fail on runtime with `NoSuchMethodError`. For example, code:
```java
NettyServerBuilder.forPort(80).directExecutor();
```
Will fail with
> `java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractServerImplBuilder
io.grpc.netty.NettyServerBuilder.directExecutor()'`
**Affected classes**
Class `AbstractServerImplBuilder` is deleted, and no longer in the
class hierarchy of the server builders:
- `io.grpc.netty.NettyServerBuilder`
- `io.grpc.inprocess.InProcessServerBuilder`
When a memory leak occurs, it is really helpful to have access records
to understand where the buffer was being held when it leaked. retain()
when we create the NettyReadableBuffer already creates an access record
the ByteBuf, so here we track when the ByteBuf is passed to another
thread.
See #8330
Most changes are migrating from conventions to the equivalent
extensions. JMH, AppEngine, and Jib plugins trigger future compatibility
warnings with `--warning-mode all`.
The movement of configurations was to allow sourceSets to create the
configurations and then we just configure them. When configurations were
before sourceSets, we'd implicitly create the configuration.
The examples were _not_ updated to the newer Gradle, although the
non-Android examples work with the newer Gradle. The Android examples
use an older Android Gradle Plugin which will need to be upgraded first.
https://github.com/grpc/grpc-java/issues/10445
* Eliminate NPE by skipping further processing when stream is defined, but doesn't have a property for streamKey (header processing identified an error)
Fixes#10364
* Add unit test for missing content type
Ciphers have been "fast enough" for testing since early Java 8 updates;
we haven't needed to override ciphers since we dropped Java 7 support.
Java 8u252 had ALPN, so Conscrypt or Jetty ALPN hasn't been necessary
for basic testing for a while. We still want specialized testing for
Conscrypt, but only tests testing Conscrypt need to care.
This avoids the (often missing) evaluationDependsOn and fixes using
results from other projects without propagating those through
Configuration. It also reduces the number of useless classes pulled in
by down-stream tests, reducing the probability of rebuilds.
The expectation of fixtures is they help testing down-stream code that
use the classes in main. That applies to all the classes here except for
FakeClock and StaticTestingClassLoader. It would also apply to many
internal classes in grpc-testing, but let's consider cleaning that up
future work.
The pinning is unreliable in Maven and ignored by Gradle. I'm not at all
convinced that we are pinning/not pinning in appropriate projects. The
pinning also serves less of a purpose since we started encouraging the
BOM and grpc-netty-shaded. Netty's HTTP/2 API has also become somewhat
stable compared to its earlier history. If we notice an up-tick in
version skew, we can reinstate it.
The pinning is annoying in the build.gradle code and causes Maven/Gradle
to download the version list once a day, which can be troublesome to
users unaware of how to tell the tools to work offline.
It also opens our users to platform issues like seen in #10043
and #10086 where Maven Central's version list was incorrectly generated.
Or like #9664 where Gradle Plugin's repository caches packages from
JCenter but the version list is not as cachable so exposed us to JCenter
instability.
This fixes#8357, by way of "we think we won't worry any more." See
90db93b9 when it was originally introduced. And issues
like #8337, #3634.
TlsTesting.loadCert() is a public API and so should be preferred over
our internal utility. It avoids creating a temp file that has to be
deleted by a shutdown hook. Usages that needed a file were not migrated.
* Made dependency on build.gradle in protobuf tasks relative so as to make downstream tasks cacheable
* Made transformer cacheable
* Add property name to path to root build.gradle
Co-authored-by: Nelson Osacky <nelson@osacky.com>
Full end to end implementation of gRPC server as a Servlet including tests and examples
Co-authored-by: Penn (Dapeng) Zhang <zdapeng@google.com>
Co-authored-by: Chengyuan Zhang <chengyuanzhang@google.com>
ExpectedException is deprecated, so I fixed the new warnings. However,
we are still using ExpectedException many places and had previously
supressed the warning. See
https://github.com/grpc/grpc-java/issues/7467 . I did not fix those
existing instances that had suppressed the warning, since it is
unrelated to the upgrade and we have been free to fix them at any time
since we dropped Java 7.
* Revert "Revert "netty:Auto adjust BDP ping frequency (#9650)" (#9821)"
This reverts commit a2bbe84198.
* Eliminate half RTT delay in sending BDP Pings when starting up.
* Eliminate delay for bdp ping when current read would push us over the threshold.
Netty is known to not use SSL hostname verfication by default[1], although
this is enabled for gRPC channels. This change validates that
functionality by porting a test with a similar purpuse for OkHttp.
* [1]: https://github.com/netty/netty/issues/8537
Trying to upgrade Gradle to 7.6 improved the checkstyle plugin such that
it appears to have been running in new occasions. That in turn exposed
us to https://github.com/checkstyle/checkstyle/issues/5088. That bug was
fixed in 8.28, which also fixed lots of other bugs. So now we have
better checking and some existing volations needed fixing. Since the
code style fixes generated a lot of noise, this is a pre-fix to reduce
the size of a Gradle upgrade.
I did not upgrade past 8.28 because at some point some other bugs were
introduced, in particular with the Indentation module. I chose the
oldest version that had the particular bug impacting me fixed. Upgrading
to this old-but-newer version still makes it easier to upgrade to a
newer version in the future.
If an artifact on Maven Central exposes a type from gRPC on its API
surface, then consumers of that artifact need that gRPC API in the
compile classpath. Bazel handles this by making hjars for transitive
dependencies, but if the dependencies are runtime_deps then Bazel won't
generate hjars containing the needed symbols.
We don't export netty-shaded because the classes already don't match
Maven Central. If an artifact on Maven Central is exposing a
netty-shaded class on its API surface, it wouldn't work anyway since the
class simply doesn't exist for the Bazel build.
Fixes#9772
Creates "Adaptive" cumulator: cumulate ByteBuf's by dynamically switching between merge and compose strategies.
This cumulator applies a heuristic to make a decision whether to track a reference to the buffer with bytes received from the network stack in an array ("zero-copy"), or to merge into the last component (the tail) by performing a memory copy.
It is necessary as a protection from a potential attack on the COMPOSITE_CUMULATOR. Consider a pathological case when an attacker sends TCP packages containing a single byte of data, and forcing the cumulator to track each one in a separate buffer. In this case we'll be paying a memory overhead for each buffer, as well as extra compute to read the cumulation.
Implemented heuristic establishes a minimal threshold for the total size of the tail and incoming buffer, below which they are merged. The sum of the tail and the incoming buffer is used to avoid a case where attacker alternates the size of data packets to trick the cumulator into always selecting compose strategy.
Merging strategy attempts to minimize unnecessary memory writes. When possible, it expands the tail capacity and only copies the incoming buffer into available memory. Otherwise, when both tail and the buffer must be copied, the tail is reallocated (or fully replaced) with a new buffer of exponentially increasing capacity (bounded to minComposeSize) to ensure runtime O(n^2) amortized to O(n).
Note: this reintroduces https://github.com/grpc/grpc-java/pull/7532, addressing the subtle issue (ref b/155940949) with `CompositeByteBuf.component()` indexes getting out of sync, which results in the merge operation producing broken buffers.
No logic changes, just cleans up warnings to make spotting real problems easier.
Remove "public" declarations on interfaces
Remove duplicate semicolons (Java lines ending in ";;")
Remove unneeded import
Change non-javadoc comment to not start with "/**"
Remove unneeded explicit type declarations from generics
Fix broken javadoc links
%s is fairly safe (requires a Formattable to use Locale), so %d is the
main risk item. Places that really didn't need to use String.format()
were converted to plain string concatenation. Logging locations were
generally converted to using the log infrastructure's delayed
formatting, which is generally locale-sensitive but we're okay with
that. That wasn't done in okhttp, however, because Android frequently
doesn't use MessageFormat so we'd lose the parameters. Everywhere else
was explicitly defined to be Locale.US, to be consistent independent of
the default system locale.
This can avoid creating an additional 736 tasks (previously 502 out of
1591 were not created). That's not all that important as the build time
is essentially the same, but this lets us see the poor behavior of the
protobuf plugin in our own project and increase our understanding of how
to avoid task creation when developing the plugin. Of the tasks still
being created, protobuf is the highest contributor with 165 tasks,
followed by maven-publish with 76 and appengine with 53. The remaining
59 are from our own build, but indirectly caused by maven-publish.
Both Netty and OkHttp had local and remote windows flipped. Also, the
comment in OkHttp about "not tracked" wasn't true, so make it more
accurate and provide a _hint_ of what the window may be instead of just
-1.
all: Update netty to 4.1.77.Final and netty_tcnative to 2.0.53.Final
Also switches to a non-release version of rules_jvm_external to allow Bazel build to work with artifact classifiers.
This moves our depedencies into a plain file that can be read and
updated by tooling. While the current tooling is not particularly better
than just using gradle-versions-plugin, it should put us on better
footing. gradle-versions-plugin is actually pretty nice, but will be
incompatible with Gradle 8, so we need to wait a bit to see what the
future holds.
Left libraries as an alias for libs to reduce the commit size and make
it easier to revert if we don't end up liking this approach.
We're using Gradle 7.3.3 where it was an incubating fetaure. But in
Gradle 7.4 is became stable.
These APIs were added to NettyServerBuilder for gRFC A8 and A9. They are
important enough that they shouldn't require using the perma-unstable
transport API to access. This change also allows using these methods
with grpc-netty-shaded.
Fixes#8991
The transport should be usable with non-`unix:` name resolvers. As long
as the name resolver returns the correct socket address type, things
should work fine.
* netty: implement UdsNameResolver and UdsNettyChannelProvider
When the scheme is "unix:" we get the UdsNettyChannelProvider to
create a NettyChannelBuilder with DomainSocketAddress type and
other related params needed for UDS sockets
* api: add support for SocketAddress types in ManagedChannelProvider
also add support for SocketAddress types in NameResolverProvider
Use scheme in target URI to select a NameRseolverProvider and get
that provider's supported SocketAddress types.
implement selection in ManagedChannelRegistry of appropriate
ManagedChannelProvider based on NameResolver's SocketAddress types
The previous code assumed that only gRPC would be using these methods.
But twice now Netty has made a change (generally relating to security)
that used a method for pseudo headers that previously wasn't supported.
Let's stop the whack-a-mole and just implement them all.
This restores compatibility with Netty 4.1.75.Final. Fixes#8981
In core, add a new enum element to `RpcProgress` for the case that the stream is closed even before anything leaves the client. `RetriableStream` will do unlimited transparent retry for this type of `RpcProgress` since they are local-only.
In netty, call `tranportReportStatus()` for pending streams on failure.
Also fixes#8394
This fixes a soon-to-be compile error via ErrorProne.
Alternatively, we could use assertThrows() instead of
@Test(expected = ...), but grpc doesn't yet require Java 8.
Previous versions of error prone were incompatible with Java 17 javac.
In grpc-api, errorprone is now api dependency because it is on a public
API. I was happy to see that Gradle failed the build without the dep
change, although the error message wasn't super clear as to the cause.
It seems that previously -PerrorProne=false did nothing. I'm guessing
this is due to a behavior change of Gradle at some point. Swapping to
using the project does build without errorProne, although the build
fails with Javac complaining certain classes are unavailable. It's
unclear why. It doesn't seem to be caused by the error-prone plugin.
I've left it failing as a pre-existing issue.
ClientCalls/ServerCalls had Deprecated removed from some methods because
they were only deprecated in the internal class, not the API. And with
Deprecated, InlineMeSuggester complained.
I'm finding InlineMeSuggester to be overzealous, complaining about
package-private methods. In time we may figure out how to use it better,
or we may request changes to the checker in error-prone.
These changes make the build compatible with Gradle 7, except for
Android which requires plugin updates.
I removed animalsniffer from binder because it did nothing (as there
were no signatures) and it was failing after setting toolVersion. It
failed because animalsniffer is only compatible with java plugin. After
this change I put the withId(animalsniffer) loading inside the
withId(java) to avoid a plugin ordering failure. That made it safe again
for binder to load animalsniffer, but it is still best to remove the
plugin from binder as it is misleading.
I did not upgrade Android plugin versions as newer versions (even 3.6)
require dealing with androidx (#8421).
In refactoring described in #7211, the implementation of #maxInboundMessageSize(int)
(and its corresponding field) were pulled down from internal AbstractManagedChannelImplBuilder
to concrete classes that actually enforce this setting. For the same reason, it wasn't ported
to ManagedChannelImplBuilder (the #delegate()).
Then AbstractManagedChannelImplBuilder was brought back to fix ABI backward compatibility,
and temporarily turned into a ForwardingChannelBuilder, ref PR #7564. Eventually it will
be deleted, after a period with "bridge" ABI solution introduced in #7834.
However, restoring AbstractManagedChannelImplBuilder unintentionally made ABI of
pre-refactoring builds expect it to be a method of AbstractManagedChannelImplBuilder,
and not concrete classes, ref #8313.
The end goal is to keep #maxInboundMessageSize(int) only in concrete classes that enforce it.
To fix method's ABI, we temporary reintroduce it to the original layer it was removed from:
AbstractManagedChannelImplBuilder. This class' only intention is to provide short-term
ABI compatibility. Once we move forward with dropping the ABI, both fixes are no longer
necessary, and both will perish with removing AbstractManagedChannelImplBuilder.
This fixes data race described in #8565.
We are doubtful whether checking closed in isReady() is necessary (#3201 might be a requirement), but it was easier to just maintain the existing behavior than think heavily about it.
Although this is part of HTTP/2 and should have already been handled
already, it was noticed as part of RBAC work to avoid matching
hop-by-hop headers. See gRFC A41.
Also add a warning if creating Metadata.Key for "Connection". Use this
to try to help diagnose a client if it happens to blindly copy headers
from HTTP/1, as PROTOCOL_ERROR is hard to debug.
This rolls-forward 6e89919 after it was reverted in 7669656, now that
the test proxy has been fixed.
We want to know the single, unambiguous authority for the request. If
there is no authority, we use host instead. While authority would be
most typical for HTTP/2, requests proxied from HTTP/1 may use host
instead of authority.
This is generally useful, but the impetus is RBAC. See gRFC A41.
Although this is part of HTTP/2 and should have already been handled
already, it was noticed as part of RBAC work to avoid matching
hop-by-hop headers. See gRFC A41.
Also add a warning if creating Metadata.Key for "Connection". Use this
to try to help diagnose a client if it happens to blindly copy headers
from HTTP/1, as PROTOCOL_ERROR is hard to debug.
Rebased PR #8343 into the first commit of this PR, then (the 2nd commit) reverted the part for metric recording of retry attempts. The PR as a whole is mechanical refactoring. No behavior change (except that some of the old code path when tracer is created is moved into the new method `streamCreated()`).
The API change is documented in go/grpc-stats-api-change-for-retry-java
Nginx and C core don't do graceful GOAWAY and retries have matured such
that transparent retries may soon be on by default. Refining the
workaround thus can reduces error rate for users.
Fixes#8310
This has been wrong since the introduction of the code in df357cb8.
Noticed as part of https://github.com/grpc/grpc-go/pull/4491 . The error
text is generally ASCII, so this probably doesn't matter much.
We used to have two ClientStreamListener.closed() methods. One is simply calling the other with default arg. This doubles debugging (e.g. #7921) and sometimes unit testing work. Deleting the 2-arg method to cleanup.
This PR is purely refactoring.
This is essentially re-adding #5901 after its behavior was "reverted"
in #8242. At the time grpc-core was the only dependency, so this change
only applies to grpc-core to make it a tiny bit easier to swap to
runtime scope in the future.
failOnVersionConflict has never been good for us. It is equivalent to
Maven dependencyConvergence which we discourage our users to use because
it is too tempermental and _creates_ version skew issues over time.
However, we had no real alternative for determining if our deps would be
misinterpeted by Maven.
failOnVersionConflict has been a constant drain and makes it really hard
to do seemingly-trivial upgrades. As evidenced by protobuf/build.gradle
in this change, it also caused _us_ to introduce a version downgrade.
This introduces our own custom requireUpperBoundDeps implementation so
that we can get back to simple dependency upgrades _and_ increase our
confidence in a consistent dependency tree.
Previously it required manually listing the direct deps of grpc-netty
which is error-prone as evidinced by the fact that we were missing
multiple deps (guava, perfmark-api). This didn't cause a problem because
grpc-core happens to bring in these same deps.
This adds support for creating a Netty Channel with SocketAddress and ChannelCredentials.
This aligns with NettyServerBuilder.forAddress(SocketAddress address, ServerCredentials creds).
joybestourous
Eric Anderson
Ulf Adams
Benjamin Fedorka
Patrick Koenig
sanjaypujare
amirhadadi
Sergii Tkachenko
Larry Safran
Stephane Landelle
Terry Wilson
Benjamin Peterson
Carl Mastrangelo
Kun Zhang
Larry Safran
Asaf Flescher
Colin Alworth
Stefan Puhlmann
yifeizhuang
Terry Wilson
Kurt Alfred Kluever
nafeabd
ZHANG Dapeng
ZhenLian
Daniel Zou
zpencer
Nick Ufer
cfredri4