This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-grpc.git
The following commit(s) were added to refs/heads/main by this push:
new 5d0a3446 Replace the resolver's assert with an
IllegalArgumentException (#873)
5d0a3446 is described below
commit 5d0a3446ad77639ea050a4828b1ae4abfad4a880
Author: PJ Fanning <[email protected]>
AuthorDate: Tue Sep 1 10:10:08 2026 +0100
Replace the resolver's assert with an IllegalArgumentException (#873)
Motivation:
The pekko-http transport resolver checked the host it was asked to resolve
against the configured `override-authority` with `assert`. Two problems:
`assert` throws `AssertionError`, which is an `Error` and not an
`Exception`, so
it passes straight through a handler that catches `Exception` and is not
what a
caller inspecting a failed connection would expect to match on.
`assert` is also elided entirely under `-Xdisable-assertions`. That is the
calling application's compiler flag, not this project's, so the check
silently
disappears in a build this project does not control.
The message was also just "assertion failed", naming neither the host nor
the
authority it was compared against.
Modification:
Extracted `checkAuthority`, which throws an `IllegalArgumentException`
naming
both hosts and the client it belongs to. Extracting it also gives the check
a
seam a test can reach; it previously lived inside a closure passed to
`ClientTransport.withCustomResolver`.
Result:
The check runs regardless of the caller's compiler flags, fails with an
exception rather than an error, and says what mismatched.
Tests:
- 3 cases in `PekkoHttpClientUtilsSpec`: a matching authority, no configured
authority, and a mismatch asserting the failure is an `Exception` and
names
both hosts
- Confirmed the guard bites: restoring `assert` fails the mismatch case,
since
`AssertionError` is not an `Exception`
- sbt "runtime/testOnly ...PekkoHttpClientUtilsSpec" - 12 passed
- sbt "runtime/mimaReportBinaryIssues" - passed
- sbt scalafmtAll scalafmtSbt - applied
- sbt "runtime/test" - not run locally, left to CI
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../pekko/grpc/internal/PekkoHttpClientUtils.scala | 23 ++++++++++++++---
.../grpc/internal/PekkoHttpClientUtilsSpec.scala | 30 +++++++++++++++++++++-
2 files changed, 49 insertions(+), 4 deletions(-)
diff --git
a/runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtils.scala
b/runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtils.scala
index 068c8f34..43fe647f 100644
---
a/runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtils.scala
+++
b/runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtils.scala
@@ -72,9 +72,7 @@ object PekkoHttpClientUtils {
@volatile var roundRobin: Int = 0
val clientConnectionSettings =
ClientConnectionSettings(sys).withTransport(ClientTransport.withCustomResolver((host,
_) => {
- settings.overrideAuthority.foreach { authority =>
- assert(host == authority)
- }
+ checkAuthority(host, settings)
settings.serviceDiscovery.lookup(settings.serviceName,
settings.resolveTimeout).map { resolved =>
if (resolved.addresses.isEmpty)
throw new IllegalStateException(
@@ -223,6 +221,25 @@ object PekkoHttpClientUtils {
}
}
+ /**
+ * INTERNAL API
+ *
+ * Checks that the host being resolved is the authority this client was
configured for.
+ *
+ * An `IllegalArgumentException` rather than `assert`: an `AssertionError`
is an `Error`, so it
+ * passes through handlers that catch `Exception`, and `assert` is elided
entirely under
+ * `-Xdisable-assertions`, which is the calling application's compiler flag
to set rather than
+ * ours. The message names both hosts, where `assert` reported only
"assertion failed".
+ */
+ @InternalApi
+ private[internal] def checkAuthority(host: String, settings:
GrpcClientSettings): Unit =
+ settings.overrideAuthority.foreach { authority =>
+ if (host != authority)
+ throw new IllegalArgumentException(
+ s"Unexpected host [$host] for gRPC client '${settings.serviceName}',
" +
+ s"expected the configured override-authority [$authority]")
+ }
+
/**
* INTERNAL API
*
diff --git
a/runtime/src/test/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtilsSpec.scala
b/runtime/src/test/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtilsSpec.scala
index 26c15539..b3dfe42c 100644
---
a/runtime/src/test/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtilsSpec.scala
+++
b/runtime/src/test/scala/org/apache/pekko/grpc/internal/PekkoHttpClientUtilsSpec.scala
@@ -18,7 +18,7 @@ import scala.concurrent.duration._
import org.apache.pekko
import pekko.actor.ActorSystem
-import pekko.grpc.GrpcResponseMetadata
+import pekko.grpc.{ GrpcClientSettings, GrpcResponseMetadata }
import pekko.grpc.scaladsl.headers.PercentEncoding
import pekko.http.scaladsl.model.HttpEntity.Strict
import pekko.http.scaladsl.model._
@@ -38,6 +38,34 @@ class PekkoHttpClientUtilsSpec extends
TestKit(ActorSystem()) with AnyWordSpecLi
implicit val patience: PatienceConfig =
PatienceConfig(5.seconds, Span(100, org.scalatest.time.Millis))
+ "checkAuthority" should {
+
+ "accept the configured override-authority" in {
+ val settings = GrpcClientSettings.connectToServiceAt("example.com",
443).withOverrideAuthority("auth.example")
+
+ noException should be thrownBy
PekkoHttpClientUtils.checkAuthority("auth.example", settings)
+ }
+
+ "accept any host when no authority is configured" in {
+ val settings = GrpcClientSettings.connectToServiceAt("example.com", 443)
+
+ noException should be thrownBy
PekkoHttpClientUtils.checkAuthority("anything.example", settings)
+ }
+
+ "reject a mismatched host with an Exception, not an Error" in {
+ // `assert` threw AssertionError, which is an Error and so passes
straight through a
+ // `catch NonFatal` or `catch Exception` handler
+ val settings = GrpcClientSettings.connectToServiceAt("example.com",
443).withOverrideAuthority("auth.example")
+
+ val thrown = the[IllegalArgumentException] thrownBy
+ PekkoHttpClientUtils.checkAuthority("other.example", settings)
+
+ thrown shouldBe a[Exception]
+ thrown.getMessage should include("other.example")
+ thrown.getMessage should include("auth.example")
+ }
+ }
+
"The conversion from HttpResponse to Source" should {
"map a strict 404 response to a failed stream" in {
val response =
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
