BaseUri name is misleading — overloaded as path-prefix and as URL across consumers #32

New issue

Open

opened 2026-06-12 05:22:50 +00:00 by mprihoda · 0 comments

mprihoda commented

2026-06-12 05:22:50 +00:00

(Migrated from github.com)

Summary

works.iterative.tapir.BaseUri carries two different semantics depending on which iw-support
consumer reads it, and the type signature (case class BaseUri(value: Option[Uri])) doesn't
distinguish them. The current name promises a URL; some consumers treat it as one, others as a
mount-path prefix. This is the root cause behind the Pac4j composition bug discussed in #31
(closed).

How each consumer interprets it

Consumer	Source	Semantic
`BlazeHttpServer.withBaseUri`	`server/http/.../impl/blaze/BlazeHttpServer.scala:13-16`	Path prefix (used as `Router(u.toString -> routes)` — concatenated as a path)
`ViteSupport.withBaseUri`	`server/http/.../ViteSupport.scala:43,96`	Path prefix (asset URL roots)
`Pac4jConfigFactory` (0.1.15+)	`server/http/.../impl/pac4j/Pac4jConfigFactory.scala:34,43`	Path prefix — composed as `urlBase + baseUri.toString + callbackBase + "/callback"` and used as cookie `Path` attribute
`ConsulKeyValueStore`	`hashicorp/jvm/.../ConsulKeyValueStore.scala:62`	URL (Consul server address — `BaseUri(addr)`)
Scala.js `fromLocation`	`tapir/js/.../BaseUriPlatformSpecific.scala:6,13`	URL (derived from `window.location`)
`LiveClientEndpointFactory.default`	`tapir/shared/.../LiveClientEndpointFactory.scala:56,61`	URL (used as base for absolute tapir client requests)

Why this is a bug-attractor

A natural-looking usage:

val baseUri = BaseUri(cfg.urlBase)  // where urlBase is the public origin, e.g. "https://app.example.com"
new Pac4jConfigFactory(baseUri, cfg, dispatcher)

compiles cleanly (the String constructor on BaseUri accepts anything sttp.model.uri"" can
parse) but produces a malformed URL at runtime. Pac4jConfigFactory composes
urlBase + baseUri.toString + callbackBase + "/callback" → for the example above,
"https://app.example.comhttps://app.example.com/auth/oidc/callback". The same value also
becomes the session-cookie Path attribute (Path=https://app.example.com).

The original Phase 2 implementer of platform PLATFORM-210 hit this exact trap — the parameter is
called baseUri, the type accepts a String, so passing the public URL felt like the right
thing. Existing iw-support projects don't hit it because they either leave BASEURI unset
(BaseUri(None) → fold returns "/" → composition works modulo a tolerable double slash) or
set it to a literal mount path like /admin.

Suggested fix

Two reasonable directions:

Rename to match the semantic. BaseUri → BasePath (or MountPath) for the HTTP-server
stack consumers. The type stays case class BasePath(value: Option[Uri]) but the name
forbids the URL interpretation at the call site.
Split into two types. Introduce a separate PublicOrigin (or Origin) concept for the
public-URL consumers (ConsulKeyValueStore, Scala.js fromLocation, tapir client factory).
Each consumer takes the type that matches its intent.

Option 2 is structurally cleaner but ripples through more call sites and downstream apps.
Option 1 is a low-impact rename that closes the most common pitfall.

A BasePath.fromString smart constructor that rejects values containing :// would also help
— it would have caught the platform bug at construction time. With the current BaseUri(String)
constructor accepting any input, the type system is too permissive.

Context

Closed PR #31 attempted to fix this in Pac4j by validating the BaseUri shape at the
factory boundary and removing urlBase from Pac4jSecurityConfig — operating on the
hypothesis that BaseUri was the canonical public-URL source. Empirical wiring proved that
was wrong: setting BASEURI=http://localhost:8090 to satisfy Pac4j caused
BlazeHttpServer.withBaseUri to mount routes under /http://localhost:8090/*. The two
consumers genuinely want different things.
The PLATFORM-210 platform-side fix went a different way: keep urlBase in
Pac4jSecurityConfig, read BaseUri.config from env with the path semantic, and never
construct BaseUri(cfg.urlBase). That works against unmodified iw-support 0.1.15 — but the
underlying API trap remains.

Out of scope for this issue

The Pac4j-specific URL/cookie composition is correctly handled today provided callers respect
the unwritten "BaseUri is a path" contract. That's the safety-on-paper status quo this issue is
asking to make explicit through naming.

## Summary `works.iterative.tapir.BaseUri` carries two different semantics depending on which iw-support consumer reads it, and the type signature (`case class BaseUri(value: Option[Uri])`) doesn't distinguish them. The current name promises a URL; some consumers treat it as one, others as a mount-path prefix. This is the root cause behind the Pac4j composition bug discussed in #31 (closed). ## How each consumer interprets it | Consumer | Source | Semantic | |---|---|---| | `BlazeHttpServer.withBaseUri` | `server/http/.../impl/blaze/BlazeHttpServer.scala:13-16` | Path prefix (used as `Router(u.toString -> routes)` — concatenated as a path) | | `ViteSupport.withBaseUri` | `server/http/.../ViteSupport.scala:43,96` | Path prefix (asset URL roots) | | `Pac4jConfigFactory` (0.1.15+) | `server/http/.../impl/pac4j/Pac4jConfigFactory.scala:34,43` | Path prefix — composed as `urlBase + baseUri.toString + callbackBase + "/callback"` and used as cookie `Path` attribute | | `ConsulKeyValueStore` | `hashicorp/jvm/.../ConsulKeyValueStore.scala:62` | URL (Consul server address — `BaseUri(addr)`) | | Scala.js `fromLocation` | `tapir/js/.../BaseUriPlatformSpecific.scala:6,13` | URL (derived from `window.location`) | | `LiveClientEndpointFactory.default` | `tapir/shared/.../LiveClientEndpointFactory.scala:56,61` | URL (used as base for absolute tapir client requests) | ## Why this is a bug-attractor A natural-looking usage: ```scala val baseUri = BaseUri(cfg.urlBase) // where urlBase is the public origin, e.g. "https://app.example.com" new Pac4jConfigFactory(baseUri, cfg, dispatcher) ``` compiles cleanly (the `String` constructor on `BaseUri` accepts anything `sttp.model.uri""` can parse) but produces a malformed URL at runtime. `Pac4jConfigFactory` composes `urlBase + baseUri.toString + callbackBase + "/callback"` → for the example above, `"https://app.example.comhttps://app.example.com/auth/oidc/callback"`. The same value also becomes the session-cookie `Path` attribute (`Path=https://app.example.com`). The original Phase 2 implementer of platform PLATFORM-210 hit this exact trap — the parameter is called `baseUri`, the type accepts a `String`, so passing the public URL felt like the right thing. Existing iw-support projects don't hit it because they either leave `BASEURI` unset (`BaseUri(None)` → fold returns `"/"` → composition works modulo a tolerable double slash) or set it to a literal mount path like `/admin`. ## Suggested fix Two reasonable directions: 1. **Rename to match the semantic.** `BaseUri` → `BasePath` (or `MountPath`) for the HTTP-server stack consumers. The type stays `case class BasePath(value: Option[Uri])` but the name forbids the URL interpretation at the call site. 2. **Split into two types.** Introduce a separate `PublicOrigin` (or `Origin`) concept for the public-URL consumers (`ConsulKeyValueStore`, Scala.js `fromLocation`, tapir client factory). Each consumer takes the type that matches its intent. Option 2 is structurally cleaner but ripples through more call sites and downstream apps. Option 1 is a low-impact rename that closes the most common pitfall. A `BasePath.fromString` smart constructor that rejects values containing `://` would also help — it would have caught the platform bug at construction time. With the current `BaseUri(String)` constructor accepting any input, the type system is too permissive. ## Context - Closed PR #31 attempted to fix this in Pac4j by validating the `BaseUri` shape at the factory boundary and removing `urlBase` from `Pac4jSecurityConfig` — operating on the hypothesis that `BaseUri` was the canonical public-URL source. Empirical wiring proved that was wrong: setting `BASEURI=http://localhost:8090` to satisfy Pac4j caused `BlazeHttpServer.withBaseUri` to mount routes under `/http://localhost:8090/*`. The two consumers genuinely want different things. - The PLATFORM-210 platform-side fix went a different way: keep `urlBase` in `Pac4jSecurityConfig`, read `BaseUri.config` from env with the path semantic, and never construct `BaseUri(cfg.urlBase)`. That works against unmodified iw-support 0.1.15 — but the underlying API trap remains. ## Out of scope for this issue The Pac4j-specific URL/cookie composition is correctly handled today provided callers respect the unwritten "BaseUri is a path" contract. That's the safety-on-paper status quo this issue is asking to make explicit through naming.