Our tests assume localhost is in /etc/hosts or uses some other form of
local-only resolution. But that wouldn't apply to "host". What was
happening is this was causing a DNS resolution, which would fail, and
the InetSocketAddress would be "unresolved". Thus, the equivalent and
faster code would be `InetSocketAddress.createUnresolved("host", 1234)`.
But there doesn't seem to be any reason to avoid localhost in this test,
so swap to the more typical solution instead.
This should avoid flakes like:
```
io.grpc.okhttp.OkHttpClientTransportTest > invalidAuthorityPropagates FAILED
org.junit.runners.model.TestTimedOutException: test timed out after 10 seconds
at java.base@11.0.17/java.net.Inet6AddressImpl.lookupAllHostAddr(Native Method)
at java.base@11.0.17/java.net.InetAddress$PlatformNameService.lookupAllHostAddr(InetAddress.java:929)
at java.base@11.0.17/java.net.InetAddress.getAddressesFromNameService(InetAddress.java:1529)
at java.base@11.0.17/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:848)
at java.base@11.0.17/java.net.InetAddress.getAllByName0(InetAddress.java:1519)
at java.base@11.0.17/java.net.InetAddress.getAllByName(InetAddress.java:1378)
at java.base@11.0.17/java.net.InetAddress.getAllByName(InetAddress.java:1306)
at java.base@11.0.17/java.net.InetAddress.getByName(InetAddress.java:1256)
at java.base@11.0.17/java.net.InetSocketAddress.<init>(InetSocketAddress.java:220)
at app//io.grpc.okhttp.OkHttpClientTransportTest.invalidAuthorityPropagates(OkHttpClientTransportTest.java:1687)
```
Previously, untrustedServer_fails could have been failing for the same
reason as unmatchedServerSubjectAlternativeNames_fails. The
implementation could have been broken and not checking the cert chain
but still checking the hostname. We'd either need to override the
authority to match the badserver cert or use the normal server
certificates. It is best to use the normal server certificates as
mtls_succeeds confirms the configuration is correct and so our test is
failing for the right reason.
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.
When allocating bytes to streams within a flow control window we always
go through the streams in the same order. This can lead to large streams
hogging all the bytes and a smaller one down the list getting starved
out. This change shuffles the stream array to lower the chance of this
happening.
Fixes#9089
%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 reverts commit c1abc7f8ac. It
produced compilation issues inside Google. I strongly suspect it isn't
this commit or gRPC's fault, but it prevents further testing until it is
resolved.
Android linters can't recognize the difference when VisibleForTesting
is used because the method has different visibility, or because
the method only intended for testing.
Because of that linter complains when VisibleForTesting methods are
used in the production code.
Ideally we want to replace or remove this annotation, as its
usage for marking altered visibility for testing purposes is
discouraged since guava v30.0.
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.
This fixes a regression introduced in e96d0477. The NullPointerException
only happens on client-side when some other error occurred during
handshaking.
I tried to add a test, but SerializingExecutor catches+logs the
exception and the expected behavior in the circumstance is that close()
is a noop. So the NPE was entirely benign other than annoying log
messages.
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.
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.
* 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
Users should be able to inject all executors. The transport shouldn't be
hard-coded to create the TIMER_SERVICE, especially since a scheduler is
already available to the builder.
This matches what we do in ManagedChannelImplBuilder and
NettyChannelBuilder. It also fixes a (probably unimportant) bug where
the factory returned from swapChannelCredentials() didn't have its
references to the executors so could not outlive the parent factory.
A substantial portion of the methods are unused. While these don't
contribute to the size of Android builds because of dead code
elimination in the build process, they still show up in static analysis
and raise questions like "when are we using MD5" or "when are we special
casing exception message text" (answer: "we're not").
In the olden days, before LB policies, transports had to accept RPCs as
soon as they were created. This hasn't been true for a very long time,
so remove the tests.
Since a978c9ed we're using real, legit code flows in the tests. This
allowed TSAN to discover that `attributes` is racy when read when
creating a new stream before the transport is ready. We could use a lock
or volatile, but the value of the attributes would still be incorrect
for any RPCs that are created before the transport is ready.
Since there's now only one test that delays the connection, I inline the
support code.
This greatly reduces the number of arguments passed to the constructor
and allows using the builder in tests to change specific arguments
without having to pass all the other arguments. It also makes it easier
to see where tests are doing something special.
While it is weird to expose fields as package-private for digging-into
in the constructor, it's actually very similar to the pattern of passing
the builder instance into the constuctor. In this case, the weirdness is
because the builder isn't a nested class of the transport and there is
an additional level of building going on (Builder and TransportFactory).
We do this pattern already in ManagedChannelImpl which only has the one
level of building.
With the completely different constructor it was hard to track which
fields were different during the test and reduced confidence. Now the
test code flows are much closer to the real-life code flows.
* okhttp: forked required files to make okhttp dep compile only
* okhttp: forked missing file to make okhttp dep compile only
* okhttp: moved url and request files to proxy packge
* okhttp: removed unused methods from forked files; fixed build
This introduces new TLS 1.2 cipher suites (#8610) and prepares the
internal okhttp implementation for TLS1.3. A new method for creating
internal ConnectionSpec was added to be able to use the newly introduced
cipher suites in the OkHttpChannelBuilder. Okhttp cipher suites
synchronized with the ones from netty.
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.
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
Carl Mastrangelo
yifeizhuang
Benjamin Peterson
Eric Anderson
yifeizhuang
Terry Wilson
Terry Wilson
Sergii Tkachenko
Jader Alcântara
sanjaypujare
beatrausch
ZHANG Dapeng
Jintao