yandrey321 opened a new pull request, #11424:
URL: https://github.com/apache/ozone/pull/11424
## What changes were proposed in this pull request?
This upgrades Jetty from `12.0.38` to `12.1.14`, moving Ozone onto the Jetty
12.1 line.
Ozone stays on the EE8 environment: `servlet-api.version` remains `4.0.9`
and no module migrates to
EE10 or EE11. Jetty publishes EE8 artifacts for every `12.1.x` release, so
there is no version floor to
clear. The property `12.0.38` occurred exactly once in the repository, so
the upgrade itself is a
one-line change to the root `pom.xml`. The three remaining changes are
fallout the upgrade produces,
each described below.
### 1. Dist license files — Jetty 12.1 splits the `jetty-ee` module
Jetty 12.1 splits `jetty-ee`, so the jar set under `share/ozone/lib` changes
even though Ozone declares
no new dependency:
| 12.0.38 | 12.1.14 |
|---|---|
| `org.eclipse.jetty:jetty-ee` | `org.eclipse.jetty:jetty-annotations` |
| | `org.eclipse.jetty.ee:jetty-ee-webapp` |
`jetty-ee-webapp` carries a *new* groupId, `org.eclipse.jetty.ee`, which did
not exist in `12.0.38`.
Neither BOM that Ozone imports manages it — `jetty-ee8-bom` contains no
`org.eclipse.jetty.ee` entry at
all. It arrives transitively because `jetty-ee8-webapp` declares it without
a version, so it follows
`jetty.version` through the pinned `jetty-ee8-webapp` pom. It resolves to a
single version, `12.1.14`,
with no conflict.
Two license files therefore need updating, which is what `dependency.sh`
exists to force:
- `hadoop-ozone/dist/src/main/license/jar-report.txt` — one entry becomes
two.
- `hadoop-ozone/dist/src/main/license/bin/LICENSE.txt` — `jetty-ee` becomes
`jetty-annotations`, and
`org.eclipse.jetty.ee:jetty-ee-webapp` is inserted between the
`org.eclipse.jetty` and
`org.eclipse.jetty.ee8` blocks, following that file's existing ordering by
groupId *segment* rather
than byte order.
Both new artifacts are `Apache-2.0`/`EPL-2.0`, so `license.sh` stays clean
and no new license category
is introduced.
A caution for anyone re-checking this: a `dependency:tree -Dincludes=...`
filter built from the *old*
coordinates is structurally blind to a groupId the upgrade introduces.
Enumerating the jars the build
actually ships (`find <dist> -name '*.jar'`) and diffing that is what
surfaced it.
### 2. `UriCompliance.Violation.FRAGMENT` — the one behavior change that
reaches Ozone
The `UriCompliance` API delta is otherwise purely additive
(`Violation.FRAGMENT` and
`UriCompliance.DEFAULT_REDIRECT`), and `UriCompliance.DEFAULT.getAllowed()`
is empty in both versions.
The one measured behavior change is an improvement:
| request target | 12.0.38 | 12.1.14 |
|---|---|---|
| `/bucket/my#key` | 200, key silently truncated to `my` | 400 |
| `/bucket/my%23key` | 200, key `my#key` | 200, key `my#key` |
| `/bucket//key` | unchanged | unchanged |
Before `FRAGMENT` existed, Jetty split such a target and handed the servlet
only the part before the
`#`, so an S3 request for the key `my#key` silently operated on `my`
instead. Rejecting the malformed
target is the safer behavior, so Ozone's relaxed compliance modes
deliberately do **not** allow
`FRAGMENT`; relaxing it was tried and reproduces the truncation.
`TestHttpServer2` gains a test pinning
this, with a raw-socket helper because `java.net.URL` treats `#` as a
client-side fragment and never
puts it on the wire.
Six other 12.1 default flips were checked and cannot affect Ozone:
`maxResponseHeaderSize`,
`relativeRedirectAllowed`, `renegotiationAllowed`, `flushOnResponseCommit`,
`setCompactPath` and
`ForwardedRequestCustomizer` have zero references repository-wide. Ozone's
entire Jetty surface is
`UriCompliance`, `SecureRequestCustomizer` and `ee8.nested.SessionHandler`.
A query-string compliance
concern was also checked and found not to exist — `%FF`, `%ZZ` and a
truncated `a%2` are accepted by
both versions.
### 3. `ProxyServer` — Jetty 12.1's proxy rewrites quoted response headers
On the upgrade, `TestS3SDK` fails exactly one assertion:
`StandardObjectHeaderTests.testObjectWriteContentLanguageAndDisposition`
expects
`attachment; filename="test.txt"` and receives
`attachment;filename=test.txt`. An A/B on the single
`jetty.version` property, same tree and same command, confirms causation —
`12.0.38` gives 232/232,
`12.1.14` gives 232 run with 1 failure.
The S3 Gateway is not involved. The endpoint those tests talk to is not a
gateway: `OzoneS3SDKTests`
builds its cluster with `MultiS3GatewayService(5)`, which hides the five
gateways behind the test-only
`ProxyServer`, an `org.eclipse.jetty.ee8.proxy.ProxyServlet`. The gateway's
own response is verbatim on
`12.1.14`, measured at three layers — the value `ObjectEndpoint` reads back
from key metadata and the
value it writes out, the value Jersey hands `addHeader`, and the container's
own `HttpFields` after the
filter chain returns. A standalone `HttpServer2` plus servlet is verbatim
too, on both GET and HEAD.
Only the hop through the proxy rewrites the value:
| Jetty | origin servlet | through the proxy |
|---|---|---|
| `12.0.38` | `attachment; filename="test.txt"` | `attachment;
filename="test.txt"` |
| `12.1.14` | `attachment; filename="test.txt"` |
`attachment;filename=test.txt` |
The mechanism is in `AbstractProxyServlet.onServerResponseHeaders`.
`12.0.38` copies each header with
`field.getValue()`. `12.1.14` adds a `filterServerResponseHeader(..,
HttpField)` overload and forwards
`field.getValueList()` joined with `","`. `HttpField.getValueList()` is a
QuotedCSV parse, which drops
optional whitespace and quoting while keeping `;param=value` — exactly the
observed transformation. It
is lossy for any non-CSV header carrying a quoted-string, so
`Content-Disposition`,
`WWW-Authenticate` and `Link` are all affected. This appears to be an
upstream Jetty defect and is
worth reporting there separately.
The change here is scoped to the harness: `ProxyServer.ProxyHandler`
overrides
`onServerResponseHeaders` to copy with `field.getValue()`, restoring the
`12.0.38` copy so the proxy
stops rewriting values the S3 Gateway already returns correctly. This keeps
the assertion meaningful
rather than relaxing it.
### Scope
Included: the version upgrade, the dist license fallout it causes, a test
pinning the `FRAGMENT`
behavior, and the harness fix needed to keep `TestS3SDK` honest.
Out of scope, each its own Jira: migrating off EE8 to EE10 or EE11, and
adopting any new 12.1 feature
(`CompressionHandler`, the new deployer, `MultiAuthenticator`).
Generated-by: Claude Code (claude-opus)
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16683
## How was this patch tested?
CI:
Existing suites plus one new unit test; no new test class or cluster
lifecycle was introduced.
- Full `mvn clean install` from a clean `~/.m2`.
- `dependency:tree` across every `org.eclipse.jetty*` groupId, including the
new
`org.eclipse.jetty.ee`: all 29 Jetty artifacts resolve to `12.1.14`,
`jetty-servlet-api` stays at
`4.0.9`, and there are zero `ee9`/`ee10`/`ee11` artifacts. The only
outliers are the pre-existing
test-scope
`org.eclipse.jetty.websocket:websocket-{api,client,common}:9.4.57.v20241219`
pulled in by
`hadoop-yarn-client`, which `jetty-bom` does not manage and which this
change does not touch.
- `dependency:tree -Dincludes=javax.servlet:*,jakarta.servlet:*` — unchanged.
- Build run **without** `-Dmdep.analyze.skip`, so `analyze-only` executes at
`verify`: 58 goals clean,
no "Used undeclared dependencies".
- Unit suites for `hdds-server-framework`, `s3gateway` and `httpfs`,
including the new
`TestHttpServer2.testUnencodedFragmentRejectedRatherThanTruncatingKey`.
- `checkstyle.sh` and `rat.sh` — both clean.
- `mvn -Pdist -DskipTests package`, then `dependency.sh` and `license.sh`:
the jar-report diff
`dependency.sh` reports is applied in this PR, and `license.sh` is clean.
- `TestS3SDK` — **232 run, 0 failures, 0 errors**. Without the `ProxyServer`
fix the same suite is
232 run with 1 failure.
- Compose smoketest, `ozone` suite — `rc=0`, zero failures.
- The proxy regression was isolated with a throwaway probe that stands a
plain origin servlet behind
`ProxyServer` and reads both legs on a raw socket, run on `12.0.38` and
`12.1.14`; the mechanism was
then confirmed from `javap -c` of
`AbstractProxyServlet.onServerResponseHeaders` in both versions.
Neither probe is part of this patch.
Not run locally, left to CI: the `ozonesecure` compose suite, which
exercises the HTTPS
`SecureRequestCustomizer` path, and the httpfs compose suite.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
