Repository navigation
HADOOP-19972. Upgrade to Jetty 12 without changing the servlet namespace - #8704
joseluisll wants to merge 25 commits into
Conversation
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
64625aa to
5e7066d
Compare
|
I would like to share the report about the impact of phase C into the current codebase. I had to take some decisions, as Jetty 12.1.12 changes several behaviours with respect to Jetty 9.4. Phase C approach review — regressions to fix, and changes needing approvalContextPhase C (HADOOP-19972) moves Hadoop to Jetty 12 ee8 under the rule "no behaviour change One regression of a specific shape was already found and fixed ( Two container facts, verified from bytecode rather than from commit messages, underpin it all: A custom reason phrase genuinely worked before and genuinely cannot now. That is forced. How Part 1 — Regressions to fix1. KMS client loses its re-authentication retry on the 403 pathFunctional, silent, untested. Highest severity here.
Fix: use 2.
|
| Area | Before → after | Why it is a choice |
|---|---|---|
RestCsrfPreventionFilter |
400 text/html + custom phrase → 400 application/json envelope |
Narrower fix existed (point validateResponse's fallback at getResponseDetail); 51a0a901 predates that reader. Splits NN from DN — the Netty RestCsrfPreventionFilterHandler:148-151 is untouched, so one refusal now has two shapes. WebHdfsFileSystem changes code path: jsonParse used to reject text/html, now succeeds and reconstructs a typed RemoteException |
NetworkTopologyServlet |
200 OK truncated → 410 Gone | The commit says it is "not a Jetty 12 regression". Only change altering a status code; also buffers the whole topology in heap and adds Content-Length |
DelegationTokenAuthenticationHandler RENEW |
403 text/html → 403 application/json envelope |
Same narrower option. Clients now catch a typed AccessControlException instead of IOException. Leaves three failure shapes in one switch |
ErrorHandler all-methods bodies |
PUT/DELETE/OPTIONS/TRACE: no entity → full HTML error page | 9.4 returned Content-Length: 0; this adds a body. Compensates for the lost phrase but is not equivalent to it. Installed only on webAppContext, so /logs and /static stay bodyless — inconsistent across contexts in a way 9.4 was not |
UriCompliance |
9.4 rejected nothing; now allows exactly 3 violations | Direction is right — strictly narrower than 9.4, never wider. But: the comment justifies SUSPICIOUS_PATH_CHARACTERS by %5C alone, while the violation also re-admits %00, %0A, %0D, %09, %7F. No config knob, no documentation, no release note for a security-relevant setting |
/logs skipped on a bad log dir |
non-admin: 403 → 404 | Forced that something change (12 fails startup on a missing base resource), but skipping the context bypasses AdminAuthorizedServlet's ACL. The comment's "answers 404 either way" holds only for admins. /logs now resolves against the root webapp. Untested |
HttpServer2Metrics |
5 metrics deleted; dispatched* re-sourced |
asyncDispatches, asyncRequests, asyncRequestsWaiting, asyncRequestsWaitingMax, expires vanish from JMX (verified: five @Metric methods removed) — dashboards silently stop resolving. dispatched* now maps to getHandleTotal/Active/Time*, a changed meaning. No release note |
Correctly forced and minimal — the model the others should have followed: ImageServlet's
reason-in-body (text/plain, no re-typing, all three readers unchanged); AuthenticationFilter's
setStatus removal (server output unchanged, readers taught to look in the body);
clearContentType (restores 9.4 exactly — verified the added setCharacterEncoding(null) is a
pure state reset when no charset is outstanding); ReopenableServerConnector (inert on first bind).
Non-findings (checked — do not re-investigate)
TestHttpServermetrics assertion — not deleted, replaced withwaitForon the same
condition. Jetty 12 books the response asynchronously; timing only.TestPseudoAuthenticator— assertion moved phrase → body; server still sends the detail.KMSAuthenticationFilter.KMSResponse— still capturesmsg, so the KMS audit log keeps
the reason (Finding 2's loss is client-side only).- Web UI — no Hadoop-authored JS reads
statusText. TestRMFailover:276asserts"Not Found"— canonical, still sent.requests()did not silently flip from total to active — Jetty 12'sgetRequests()
returns_requestStats.getTotal().- Other
getResponseMessage()callers are logs or non-Jetty paths; no remaining
custom-reasonsetStatus(int,String)senders in main source.
Verification
git checkout phase-c-rebased
# Findings 1-2: no test exists for the KMS retry. Add one that drives a 403 carrying a custom
# reason through KMSClientProvider and asserts the token is reset and the call retried.
mvn -Denforced.maven.version='[3.9.0,)' -Dcheckstyle.skip=true -Dspotbugs.skip=true \
-Dmaven.test.failure.ignore=false -pl hadoop-common-project/hadoop-kms -am test
# Findings 2-5
mvn ... -pl hadoop-common-project/hadoop-common,hadoop-common-project/hadoop-auth \
test -Dtest='TestHttpExceptionUtils,TestWebDelegationToken,TestAuthenticatedURL,TestHttpServer'
# Finding 3
mvn ... -pl hadoop-hdfs-project/hadoop-hdfs -am test -Dtest='TestWebHDFS,TestWebHdfsUrl'5e7066d to
8736bca
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
f8e54b9 to
68d6433
Compare
|
💔 -1 overall
This message was automatically generated. |
68d6433 to
c75ebe5
Compare
c75ebe5 to
056c562
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
… module classpath Several modules resolved more than one Jetty release, and more than one servlet API, on a single classpath. Both combinations compile and then fail at run time, on whichever code path reaches the wrong jar. * hadoop-project excluded javax.servlet-api from jetty-server under the group org.eclipse.jetty, which jetty-server has never used, so the exclusion matched nothing and javax.servlet:javax.servlet-api stayed on the classpath beside the managed jakarta.servlet:jakarta.servlet-api. Both publish the javax.servlet packages, so which one a module compiled and ran against was decided by the order of the jars, and 73 modules carried both. An exclusion naming the real coordinates is added beside the existing one, and the same on javax-websocket-server-impl, which reaches the API again through websocket-servlet. hadoop-common now declares jakarta.servlet-api itself: HttpServer2 and the filters around it are written against javax.servlet, and the API used to arrive there only as a transitive of jetty-server. * Three modules used org.eclipse.jetty from their main sources while no pom in their inheritance chain declared Jetty: hadoop-mapreduce-client-shuffle, hadoop-yarn-server-router and hadoop-yarn-server-applicationhistoryservice. Each now declares the artifact its main sources use, without a version, and jetty-http gains the dependencyManagement entry it was missing. The Router declares jetty-webapp at compile scope, since a test scope would override the compile-scoped copy it inherits and take it off the Router's own runtime classpath. * solr-core carried nine Jetty artifacts, and a servlet API, into hadoop-yarn-applications-catalog-webapp's tests at a much older release. Those tests reach Solr only through EmbeddedSolrServer, which runs no servlet container, so none of it is needed. The exclusions are applied to solr-test-framework as well, whose own path to solr-core would otherwise reintroduce the same jars. javax.servlet:javax.servlet-api no longer ships, so its LICENSE-binary and NOTICE-binary entries go with it. One module keeps two servlet APIs, hadoop-yarn-server-timelineservice-hbase-tests, where the second arrives with HBase's own test stack. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jersey-test-framework-provider-jetty runs the Jersey tests on a Jetty of its own choosing, which is a second Jetty release on the test classpath of thirteen modules and the reason hadoop-client-runtime had to stop shipping jetty-util. The tests do not care which server answers them, so they move to jersey-test-framework-provider-jdk-http, which runs on com.sun.net.httpserver in the JDK and adds no third-party server at all. With the Jetty test container gone, hadoop-client-runtime ships jetty-util again. hadoop-client-minicluster had excluded it from hadoop-minicluster, hadoop-yarn-server-nodemanager and hadoop-yarn-server-web-proxy on the understanding that the runtime jar owns it; YARN-11793 had to break that when the test container brought jetty-util into the minicluster along a path those exclusions miss. Leaving the exclusion in place now would ship org.eclipse.jetty.util in neither jar. jetty-continuation goes the same way: the test container was the last thing pulling it in. hadoop-client-minicluster stops relocating com/sun/net/httpserver, which the JDK owns, and LICENSE-binary picks up the two artifacts that replace the Jetty provider. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
javax-websocket-server-impl brings jetty-annotations, and with it javax.annotation:javax.annotation-api 1.3.2. That is a second copy of the javax.annotation classes jakarta.annotation:jakarta.annotation-api 1.3.5 already provides through Jersey, so every classpath with both the NodeManager and Jersey carried both, and which one a module ran against was decided by the order of the jars. It is the same duplicate this change removes for javax.servlet-api, brought in the same way. Exclude it on the same javax-websocket-server-impl entry. The NodeManager, the only module that depends on it, keeps the classes through jakarta.annotation-api, and no module resolves javax.annotation-api any more apart from hadoop-maven-plugins, which gets it from Maven's own plugin API at build time. It no longer ships, so its LICENSE-binary and NOTICE-binary entries go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Jetty 12's ee8 environment depends on org.eclipse.jetty.toolchain:
jetty-servlet-api 4.0.9 (ee8.jetty.servlet.api.version in 12.1.x). Settle
on that coordinate here rather than on jakarta.servlet:jakarta.servlet-api
4.0.4, so the Jetty 12 upgrade (HADOOP-19972) does not have to change it a
second time. Both publish the same javax.servlet 4.0 classes; the Jetty
artifact also carries the web.xml schemas.
- hadoop-project manages jetty-servlet-api at ${jetty-servlet-api.version}
in place of jakarta.servlet-api 4.0.4.
- Every module that declared jakarta.servlet-api declares jetty-servlet-api.
- jersey-test-framework-core declares jakarta.servlet-api at compile scope;
it is excluded there and on the JDK HTTP provider that brings it.
- hadoop-client-minicluster excludes jetty-servlet-api where it excluded
jakarta.servlet-api, as hadoop-client-runtime ships it.
- LICENSE-binary lists jetty-servlet-api 4.0.9 under Apache 2.0 and drops
jakarta.servlet-api 4.0.4.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ax.servlet. Jetty 9.4 is end of life and 12 is the supported line. Jetty 12 ships the servlet container as a pluggable EE environment; ee8 is the one that serves javax.servlet, so the tree moves to 12 without touching the namespace, Jersey 2, or any public signature. Dependency management moves to 12.0.37. The core artifacts keep their coordinates; the servlet container moves under org.eclipse.jetty.ee8, and the HTTP/2 artifacts the catalog webapp pins gain the jetty- prefix Jetty 12 gave them. jetty-servlet-tester goes: it does not exist on the 12.x line and no module declared it. The servlet API stays where HADOOP-19970 put it, org.eclipse.jetty.toolchain:jetty-servlet-api 4.0.9, which is the artifact the ee8 modules depend on. That choice matters here and not before: it is the only servlet API jar that also carries the web.xml schemas, which ee8 resolves from whichever archive javax.servlet.Servlet came from. HttpServer2, HttpServer2Metrics, the YARN WebApps builder, the WebSocket code and the remaining embedded servers are ported to the ee8 API. The shaded client artifacts and LICENSE-binary follow the new coordinates. Four things fall out of the move: * hadoop-client-minicluster already excludes jetty-servlet-api from the Hadoop modules it pulls back in, since HADOOP-19970, but the Jersey test framework artifacts it adds back exclude the servlet API under the jakarta.servlet coordinate only, or not at all. They exclude it under both, so the 85 javax.servlet classes do not land in both shaded jars - which BanDuplicateClasses in hadoop-client-check-test-invariants catches. * Jetty 12's DefaultServlet defaults dirAllowed to true, where 9.4's init parameter had it off for this context. It is set explicitly, and asserted, so no directory listing is served from /static. * Jetty 9.4's WebSocketServlet bootstrapped itself from init(). Jetty 12's JettyWebSocketServlet expects the WebSocket components on the context already and throws otherwise, so the node manager's container shell endpoint answered 500 on upgrade. What installs them is a ServletContainerInitializer, discovered only by AnnotationConfiguration, which lives in jetty-ee8-annotations - excluded here because it drags in an asm the enforcer bans. Calling the initializer directly does the same work without the scan, and has to happen while the context is still stopped, so the WebApps builder gains a window between build and start. TestNMContainerWebSocket caught none of this because it wrapped the exchange in catch (Throwable) and logged; it now asserts the upgrade produced a session. * hadoop-common's exclusion of javax.servlet-api from jetty-util matches no edge, because Jetty 12's jetty-util depends on nothing but slf4j, and the Router pom's comment described jetty-webapp and jetty-servlet, neither of which this tree has any more. Where Jetty 12 would change what a caller or an operator sees, the port keeps trunk's behaviour: * Jetty 12 sends no custom reason phrase, so a refusal's message reaches the caller only in the error page sendError writes - and Jetty writes that page for GET, POST and HEAD only, on 9.4 and 12 alike. On trunk the places whose message a caller acts on also put it in the reason phrase. AuthenticationFilter and KMSAuthenticationFilter now mark their refusals with AuthenticationFilter.ERROR_MESSAGE_FOR_ANY_METHOD_ATTRIBUTE, and HttpServer2 installs an error handler on the webapp, /logs and /static that writes the page for a marked error whatever the method, and answers every other error as 9.4 did. It extends the ErrorPageErrorHandler a WebAppContext installs for itself, so <error-page> and <exception-type> mappings in a webapp's web.xml keep working. AuthenticationFilter keeps its setStatus(code, reason) for containers that still send the phrase; KMSAuthenticationFilter's Jetty-only setStatusWithReason is gone. * Jetty 12 refuses to start a context whose base resource does not exist. When hadoop.log.dir is not a directory, /logs keeps its context, filters and admin check without a resource base, so a non-admin is refused and an admin gets 404, for a POST as for a GET, as trunk answered. * Jetty 12's StatisticsHandler records times in nanoseconds where 9.4 used milliseconds, and has no async counters. HttpServer2Metrics converts the eight requestTime* and dispatchedTime* metrics back to the milliseconds their descriptions promise, and keeps publishing asyncDispatches, asyncRequests, asyncRequestsWaiting, asyncRequestsWaitingMax and expires, as 0, so nothing that reads those names stops resolving them. On 9.4 they were not always 0: its DefaultServlet sent static files asynchronously. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…that embed it. HBase, Hive, Spark, Ozone and Knox compile against hadoop-auth's filters and HttpServer2. Moving to Jetty 12 must not change what they see: the servlet types on that surface stay javax.servlet, and nothing on it may name a jakarta.servlet type. A test asserts it rather than leaving it to review. It fails the day the jakarta rename reaches this surface, which is the point - that day belongs to a major release, not to this change. Co-Authored-By: Claude <noreply@anthropic.com>
Jetty 9.4's ServerConnector.close() released the accept channel, so a listener could re-bind. Jetty 12's does not, and a connector that has been closed cannot be started again. Hand it a fresh channel instead. Found by pjfanning on PR apache#8653. Co-Authored-By: Claude <noreply@anthropic.com>
WebHDFS answers a bad path with its own JSON RemoteException, and its
clients parse that body. On Jetty 12 they were getting a bare 400 with no
body at all, because the connector rejects the URI before any servlet
runs: //tmp//file trips UriCompliance.Violation.AMBIGUOUS_EMPTY_SEGMENT,
which DEFAULT does not allow and Jetty 9.4 did.
Reproduced against 12.0.37 with a servlet that answers 400 with a JSON
body, driven over a raw socket so the path is sent as written:
PUT /webhdfs/v1/tmp/file -> 400 len=-1 application/json {"RemoteException"...
PUT /webhdfs/v1//tmp//file -> 400 len=0 ctype=null (empty)
The second never reached the servlet, which is why
WebHdfsFileSystem.jsonParse returns null - it returns null exactly when
Content-Length is 0 - and TestRouterWebHdfsMethods then dies on a
NullPointerException rather than reading the exception it asked for.
Only that one violation is allowed back. The ambiguities that let a
request read as one path to a filter and another to a servlet stay
rejected, and the same probe confirms it: with the violation allowed,
tmp%2Ffile is still refused and a/../../etc still never reaches the
servlet.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A file whose name contains a '%' is written on the wire as %25, and that trips UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING, which DEFAULT does not allow. Names like that are ordinary in HDFS, and YARN routes carry them: TestWebApp#testEncodedUrl asks for "@;%$", which encodes to %40%3B%25%24 and came back 400 with no body. Isolated by adding one violation at a time to DEFAULT and re-running the same two requests. The encoded semicolon is not the problem - it is the encoded percent: + AMBIGUOUS_PATH_PARAMETER @;%$ -> 400 %2F -> 400 + AMBIGUOUS_PATH_SEGMENT @;%$ -> 400 %2F -> 400 + AMBIGUOUS_PATH_ENCODING @;%$ -> ok %2F -> 400 + SUSPICIOUS_PATH_CHARACTERS @;%$ -> 400 %2F -> 400 Allowing it does not widen anything else. With both violations set, a%2Fb and a%2E%2E%2Fb are still refused, .. still cannot climb out of the context, and a%252Fb still decodes once, to the literal a%2Fb, rather than to a separator. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TestWebHdfsUrl creates files whose names contain the characters HDFS
allows and a URL does not, and two of those tests came back 400 with no
body: "Unexpected HTTP response: code=400 != 307, op=CREATE, message=Bad
Request". The name is percent-encoded on the wire, so the connector was
refusing an escape sequence rather than a character.
Which one was measured rather than guessed. Sending each character of
that filename on its own, percent-encoded, against the setting this
branch already had, exactly one is refused:
specialFile ?"\()[]_-=&+;,{}#%'`~!@$^*|<>.
^
%5C
Jetty flags an encoded backslash as suspicious because it separates paths
on Windows. On HDFS it is a character in a name like any other, and 9.4
passed it through, so SUSPICIOUS_PATH_CHARACTERS joins the two violations
already allowed.
Re-checked with all three set: a%2Fb and a%2E%2E%2Fb are still refused,
.. still cannot climb out of the context, and a%252Fb still decodes once,
to the literal a%2Fb rather than to a separator.
The three are the default of a new key, hadoop.http.uri.compliance.violations,
since this decides what reaches every servlet and an operator should be able
to narrow it. SUSPICIOUS_PATH_CHARACTERS is wider than the backslash that
needed it: it admits the encoded control characters %00, %09, %0A, %0D and
%7F too, so a deployment with no use for a backslash in a path can refuse the
lot. An empty value is Jetty's DEFAULT mode, and an unknown name stops the
server rather than being ignored, since a typo in a security setting should
not pass silently. The default is split like a configured value, so a
Configuration built without core-default.xml gets the same three.
core-default.xml documents the key, the control characters, and that adding
AMBIGUOUS_PATH_SEPARATOR accepts %2F again, which 9.4 decoded to a separator
and this default refuses.
TestWebHdfsUrl 11/11. TestHttpServer covers the strict setting, a setting
without SUSPICIOUS_PATH_CHARACTERS, an unknown name, and a Configuration
without core-default.xml.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TestWebHdfsWithRestCsrfPreventionFilter passes 32/32 on trunk and fails six times here, all on the same assertion: the exception the client raises no longer contains "Missing Required Header". The filter refuses with sendError(400, message), after putting the message in the reason phrase through Jetty's own Response.setStatusWithReason. Jetty 12 has no such method and sends no custom reason phrase. What stands in its place is the error page sendError writes, which carries the message - but Jetty writes it for GET, POST and HEAD only, and the filter refuses PUTs and DELETEs as well. The filter now calls the Servlet API's setStatus(code, message) in place of the Jetty cast, which a container that still sends a reason phrase honours - Jetty 9.4 does, and other projects run this filter on it - and Jetty 12 ignores. It also marks the request with AuthenticationFilter.ERROR_MESSAGE_FOR_ANY_METHOD_ATTRIBUTE, so that HttpServer2's error handler writes the page for a PUT or a DELETE too. The status, the sendError and the body of a GET or a POST refusal are trunk's. With the body readers later in this series, the WebHDFS test passes 32/32. Only the servlet side changes. HttpInteraction has four implementations; the two Netty ones serving the DataNode build their own responses and never went through a reason phrase, so they are left alone. TestRestCsrfPreventionFilter also checks the reason phrase and the marker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ImageServlet reported why it turned a transfer down through sendError, and the detail reached the caller in the HTTP reason phrase. Jetty 12 does not send a reason phrase, so the detail now lives only in the error page the container renders - and this servlet closes its output stream on the way out of every request, committing an empty response before that page is ever produced. The secondary namenode was left with a bare status code. The servlet now writes the reason into the body itself, which does not depend on how the container renders errors, and the three clients that read a failed transfer - Util#doGetUrl, TransferFsImage's upload, and EditLogFileInputStream - read the body before falling back to the phrase. TestCheckpoint's short-send case expects a different message now. Jetty 12 fails the servlet's own write as soon as it delivers fewer bytes than the Content-Length it announced, so the namenode reports the shortfall rather than putting a truncated body on the wire for the secondary to notice. The checkpoint still fails without corrupting the namenode, which is what the test guards. A refusal found inside a transfer reaches the servlet's own catch as well, which reports it again as "GetImage failed" or "PutImage failed" and finds the response already committed. That second report is logged at DEBUG: the client already has the reason, and Jetty logs the rethrown exception. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every server HttpServer2 builds runs QuotingInputFilter, which sets text/plain; charset=utf-8 before a request reaches its resource, so a JAX-RS resource that picks its own content type clears that first. Fourteen of them did it with setContentType(null). That is no longer enough. Jetty 12 drops the charset but remembers that one had been set explicitly, and appends the memory to the next content type - literally ";charset=null" for a type that carries no charset of its own. WebHDFS answered SETPERMISSION with "application/octet-stream;charset=null"; application/xml is affected the same way, while types that assume a charset, application/json among them, escape it. Clearing the character encoding as well resets that state. The idiom moves to JettyUtils#clearContentType so all fourteen call sites get it, and TestHttpServer asserts the header a resource that clears and re-sets its content type actually sends. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… read. An audit of every sendError, two-argument setStatus and getResponseMessage call in the tree turned up two more clients that still read the detail from the HTTP reason phrase, which Jetty 12 no longer sends: WebHdfsFileSystem, on a 401, threw AccessControlException carrying only the phrase - now always "Unauthorized" rather than what the server said. AuthenticatedURL reported an authentication failure the same way, so an operator saw "message: Forbidden" instead of the reason. Both peers already put the reason in the body: AuthenticationFilter follows its setStatus with sendError, which writes the reason into the error page. The body reader moves from Util to HttpExceptionUtils, next to the JSON envelope reader it complements, so hadoop-hdfs and hadoop-hdfs-client share one copy. hadoop-auth sits below hadoop-common and cannot reach it, so AuthenticatedURL keeps a small private one. Two findings from the same audit are deliberately left alone. NetworkTopologyServlet loses its reason the same way, but a probe against both containers shows Jetty 9.4 losing it identically - a failed topology dump answers 200 there too - so it is not this migration's regression and belongs in its own change. KMSClientProvider matches the phrase against two literals; the match is dead now, but its retry still works through the arm that does not read the phrase. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TestAppCatalogSolrClient failed with NoClassDefFoundError on org.eclipse.jetty.client.api.Request$BeginListener. That package is Jetty 9.4; Jetty 12 moved the class to org.eclipse.jetty.client. Solr 8.11.2 is compiled against Jetty 9.4, and it needs the client even here: EmbeddedSolrServer's own constructor loads HttpShardHandlerFactory, which reaches for that class. The premise this module was migrated on - that EmbeddedSolrServer runs no container so Solr's Jetty is dead weight - does not hold, and excluding Solr's Jetty left it calling a Jetty 12 that has no such API. The two lines cannot share a classpath: they share jetty-http, jetty-io and jetty-util. So this module follows Solr rather than Hadoop, on a new solr.jetty.version. That is 9.4.58.v20250814, the release trunk ships in app.war, rather than the 9.4.44 Solr 8.11.2 was built against; nothing needs 9.4.44, and the module's tests pass on 9.4.58 as they do on trunk. A local dependencyManagement block overrides the inherited pin for the five artifacts hadoop-project manages and imports the jetty-bom of the same release, so the rest of Solr's Jetty - the server side solr-core and solr-test-framework bring, and http2-server - does not arrive at 9.4.44 beside it. The http2 dependencies go back to their 9.4 coordinates, and the exclusions the upgrade added on solr-core and solr-solrj are gone. Hadoop's own Jetty 12 still arrives here through hadoop-common's ee8 artifacts, which the war never uses. That is worth a look in review: excluding it from hadoop-common alone does not remove it, because three other Hadoop dependencies carry it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The previous commit put this module on the Jetty 9.4 that Solr calls, but Hadoop's Jetty 12 still arrived through its ee8 artifacts, leaving two Jetty lines in the war with Jetty 12's ee8 classes sitting on top of Jetty 9.4's jetty-http, jetty-io and jetty-util. Nothing here touches them - the module is packaged as a war and served by someone else's container, so it never starts an HttpServer2 - but that holds only for as long as nothing does. Excluded from all four Hadoop dependencies that carry them. One is not enough: hadoop-common, hadoop-yarn-common, hadoop-yarn-services-core and hadoop-yarn-services-api each bring their own copy, so excluding on any one of them changes nothing. The module now resolves a single Jetty release, which is what HADOOP-19970 set out to give every module - here it is Solr's release rather than Hadoop's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Jetty has said 12.0.x stops receiving updates during 2026 in favour of 12.1.x, so landing a migration on 12.0 would arrive on a line that is already closing. Both lines carry the ee8 environment this change depends on, and every ee8 artifact named here exists at 12.1.12. No source change goes with it. The property and the LICENSE-binary entries carry the version; jetty-servlet-api stays at 4.0.9, which is what the ee8 parent pins in both lines. Co-Authored-By: Claude <noreply@anthropic.com>
…elope. Reading a refusal's reason from the body reached further than it should have. A response that carries the JSON envelope createServletExceptionResponse writes is sent with setStatus, not sendError, so its reason phrase was the canonical text for the status code before Jetty 12 and still is - nothing was lost in the upgrade for those, and there was no reason in the body to recover. Preferring the body anyway replaced a readable "Forbidden" with a line of JSON on every endpoint that refuses that way, the proxy-user check in DelegationTokenAuthenticationFilter among them. TestKMS caught it: testWebHDFSProxyUserKerb and testWebHDFSProxyUserSimple ask for a doAs the ACLs refuse and look for "Forbidden" in what comes back. Both readers now leave a JSON body alone and report the phrase. The body is still preferred everywhere else, which is the case they exist for: what sendError is given no longer reaches the wire as a phrase. Co-Authored-By: Claude <noreply@anthropic.com>
…acted on. Earlier commits taught the readers that log or report a failure to look in the response body, because Jetty 12 never puts a reason phrase on the wire. Three readers that act on the reason were left behind. KMSClientProvider decides whether to reset its token and re-authenticate by matching the reason against "Invalid signature" and "Anonymous requests are disallowed". Against the canonical phrase that match can no longer succeed, so a 403 out of AuthenticationFilter - what an AuthenticationException from managementOperation produces - ended the call instead of retrying it. The new TestKMS case fails without this change and passes with it. HttpExceptionUtils.validateResponse fell back to the phrase whenever the body was not the JSON envelope, which is exactly the case where the body is the only place the reason is. Reading it back needs the error stream after the JSON parse has already failed on it, so the stream is buffered and marked first, and the reader is shielded from the close the JSON parser performs on its way out. An envelope too large to rewind still parses; only the text fallback is given up, which is what happened before. WebHdfsFileSystem reported the phrase in the two arms either side of the one fixed earlier. jsonParse now rejects an incompatible content type before it opens the stream, so a body it will not read is left intact for the caller to report rather than opened and closed unread. AuthenticatedURL's body reader kept a simpler HTML stripper than the one in HttpExceptionUtils, so an error page with inline script read differently through the two paths. The duplication is forced - hadoop-common sits above hadoop-auth - but the drift was not. Both body readers also flattened Jetty's whole error page to text, so a WebHDFS mkdir refused by the CSRF filter read "message=Error 400 Missing Required Header ... URI: ... STATUS: 400 MESSAGE: ... SERVLET: ..." where trunk read "message=Missing Required Header for CSRF Vulnerability Protection". Jetty's error page, on 9.4 and on 12, carries the reason alone in its MESSAGE row, so HttpExceptionUtils and AuthenticatedURL now take that row when the page has one, which gives back exactly the text the reason phrase carried, and strip the markup of any other page as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ext. YARN-11977 added McpHttpServer after this branch was cut. It builds its context from org.eclipse.jetty.servlet, which Jetty 12 moved to org.eclipse.jetty.ee8.servlet; nothing else in it changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Jetty 9.4's SslContextFactory allowed a client to renegotiate a TLS 1.2 session; Jetty 12.1 does not, and HttpServer2 never set it. On a live cluster, a client that renegotiated and then sent a request got an answer from every HTTPS endpoint on Jetty 9.4 (NameNode, SecondaryNameNode, RM, NM, JobHistory, KMS, HttpFS), and a closed connection on Jetty 12. Keep Jetty 12's refusal, which also closes off the renegotiation denial of service of CVE-2011-1473, and add hadoop.http.ssl.renegotiation.allowed for a client that needs the old behaviour. HttpServer2 now sets the value either way, so it no longer depends on the Jetty version's default. Tests renegotiate over TLS 1.2 against a server with the default and one with the key set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HttpConfiguration.relativeRedirectAllowed exists in both versions and defaults to false on Jetty 9.4 and true on Jetty 12; HttpServer2 never set it. At Jetty 12's default sendRedirect stops resolving the location against the request, so Location carries the bare path the servlet passed - "/target/page" where trunk sent "http://host:port/target/page". Hadoop redirects through sendRedirect in the YARN web proxy (ProxyUtils, WebAppProxyServlet), AmIpFilter, the webapp Dispatcher, WebServlet and the JWT login handler. A relative Location is legal and a browser resolves it, but Hadoop also reads the header itself: WebHdfsFileSystem hands it to new URL(...), which rejects a value with no protocol. WebHDFS itself is not affected, because the NameNode builds its 307 through JAX-RS temporaryRedirect and Jersey writes that header directly, so the connector's setting never reaches it - but the idiom is Hadoop's own and it ships in a client. Nothing in Jetty 12 forces the change, so set the value either way, as the renegotiation key already does, and keep trunk's answer. Measured against the same build on Jetty 9.4.58, on a server started by HttpServer2: with the setting, the six shapes sendRedirect can be handed - a context-absolute path, a path relative to the servlet, a full URL, one with a query, one with encoded characters, and one climbing out with .. - all come back byte for byte as 9.4 sent them, the case where the Host header names a proxy included. The test reads the Location header off the wire rather than the argument sendRedirect was given. TestAmFilter and the proxy tests stub sendRedirect and record its argument, which is why nothing caught this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…structor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HfuWk9kVTUjXZZ9oQWNUVD
…elpers around it. AuthenticatedURL and HttpExceptionUtils carried two copies of the code that reads a refusal's reason from the body. hadoop-common already depends on hadoop-auth, so keep one copy there, in ResponseDetail, and call it from both. validateResponse now reads the error body once into a byte array, which replaces the mark/reset buffer, the close-shielding stream and the rewind limit. The capped reads use InputStream.readNBytes. Also: WebApps keeps a single server configurer rather than a list for one caller, remoteHost uses pattern matching, and the downstream servlet test reads its response with HttpServerFunctionalTest.readOutput. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… duplicate reader, and repeated exclusions. * HttpExceptionUtils#getResponseDetail only forwarded to ResponseDetail#of, and both are Private. Callers use ResponseDetail#of directly. * HttpExceptionUtils#textDetail repeated ResponseDetail#of on a body already read. ResponseDetail#of(byte[], HttpURLConnection) does that once, and MAX_BYTES, phrase and toPlainText go private. * Both ContainerShellWebSocket classes carried a remoteHost helper only to log a host name. They log the remote address instead. * TestDownstreamServletCompatibility scanned the public surface for jakarta.servlet types, which testServletApiOnTheClasspathIsJavax already rules out: no class naming jakarta.servlet loads without that namespace on the classpath. Two asserts that held by construction go with it. * hadoop-client-minicluster excluded jetty-servlet-api twice in a row in three dependencies. * The comment on HttpServer2's URI compliance repeated the javadoc of HTTP_URI_COMPLIANCE_VIOLATIONS_KEY and the core-default.xml description. It keeps what neither says: which ambiguities stay refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bc559f0 to
889eef6
Compare
|
💔 -1 overall
This message was automatically generated. |
|
Following up from HADOOP-19910, which was closed Won't Do because no open-source |
|
There is no target date. There is preparatory and experimental work ongoing, where jetty12-ee8 is selected as the lowest risk and more compatible option. In theory it would be posible to backport jetty12-ee8 to 3.5. Please have a look at HADOOP-19912 as the umbrella for the jetty12-ee8 work. |
Depends on (has to be merged first):
mutual: each PR clears a CI failure of the other, so neither goes green alone - merge them back to back
clears a CI failure: it edits TestDFSClientRetries.java:1309-1328, where TestDFSClientRetries fails at line 1326 in this PR's precommit, still red in the latest run
clears a CI failure: it changes McpHttpServlet, which carries the trunk spotbugs warning SE_BAD_FIELD that turns this PR red, still red in the latest run
clears a CI failure: it edits TestRouterWebServicesREST.java:1346-1370, where TestFederationWebApp fails at line 1355 in this PR's precommit, still red in the latest run
Required by (waits for this one to be merged):
mutual: each PR clears a CI failure of the other, so neither goes green alone - merge them back to back
Description of PR
Stacked on #8699 (HADOOP-19970): the first three commits are that PR. Review the last 20.
Jetty 9.4 is end of life. This moves Hadoop's web servers to Jetty 12.1.12 on the ee8 environment, which still serves
javax.servlet, so the servlet namespace, Jersey 2 and every public signature stay as they are. The move tojakarta(HADOOP-19912) remains a separate decision.What changes
org.eclipse.jetty.ee8. The servlet API becomesorg.eclipse.jetty.toolchain:jetty-servlet-api4.0.9: the samejavax.servletclasses, and the only artifact that also carries the web.xml schemas ee8 needs.HttpServer2and its metrics, the YARN web app builder, the WebSocket code, KMS, HttpFS, SLS, the YARN services API and the MCP server are ported. The shaded clients andLICENSE-binaryfollow./staticlists no directory, a stopped listener can be reopened, the NodeManager's WebSocket upgrade works,/logskeeps its admin check when the log dir is missing, web.xml error pages keep working, and the HTTP server metrics stay in milliseconds under their existing names.What still differs from trunk (for the release note; the reason phrase alone makes this Incompatible)
Unauthorizedrather thanAuthentication required.%2F,%2Esegments and malformed encodings get 400.hadoop.http.uri.compliance.violationscontrols what is let through; addingAMBIGUOUS_PATH_SEPARATORaccepts%2Fagain.hadoop.http.ssl.renegotiation.allowed=truerestores 9.4's behaviour.charset=utf-8andVaryon static files, aDateheader on error responses, and the default acceptor count.How was this patch tested?
The full module test suites pass on the fork's CI (Java 17), and the build passes on Java 21 and 25.
New tests cover the servlet contract offered to downstream (
TestDownstreamServletCompatibility: it staysjavax), the error bodies for marked refusals,/logswith a missing log dir, the metric units and names, and the two new keys.Live tests compared this branch with trunk on Jetty 9.4, request by request, and ran each version's CLI against both servers, on three clusters:
The only differences found are those listed above.
During testing, i filed this bug: MAPREDUCE-7546 (#8759)
For code changes:
declared according to the connector-specific documentation? Note: Automated CI
testing doesn't cover all cases so manual testing with cloud storage is still
required.
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
Contains content generated by Claude.
where is the name of the AI tool used.
https://www.apache.org/legal/generative-tooling.html