-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29226 Migrate to jetty 12 with EE8 and bump java servlet to 4.0.1 #6783
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java
Outdated
Show resolved
Hide resolved
hbase-it/src/test/java/org/apache/hadoop/hbase/MockHttpApiRule.java
Outdated
Show resolved
Hide resolved
hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/NamespacesResource.java
Outdated
Show resolved
Hide resolved
...shaded/hbase-shaded-check-invariants/src/test/resources/ensure-jars-have-correct-contents.sh
Show resolved
Hide resolved
hbase-it/src/test/java/org/apache/hadoop/hbase/MockHttpApiRule.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hbase-http/src/main/java/org/apache/hadoop/hbase/http/log/LogLevel.java
Outdated
Show resolved
Hide resolved
hbase-http/src/test/java/org/apache/hadoop/hbase/http/log/TestLogLevel.java
Show resolved
Hide resolved
hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java
Outdated
Show resolved
Hide resolved
hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestGetAndPutResource.java
Show resolved
Hide resolved
hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestSecureRESTServer.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
...il/src/main/resources/org/apache/hadoop/hbase/shaded/org/eclipse/jetty/webapp/webdefault.xml
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
PDavid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks, looks great! 👍
|
This PR is unblocked now. Have removed the staging repo ref to build against actual hbase thirdparty release. Let's wait for the CI result. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Will merge this after #7207 and then rebase again. |
- Also consume hbase-thridparty having the new jetty 12 ee8 module
… add details on why we need violations
… - Discrepancy between Jetty 9 and Jetty 12 when setting the base resource to a path containing ..
|
Rebased code after thirdparty PR merge, will wait for basic build to complete and then merge today. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
….1 (#7233) (#6783) Please find a detailed summary of all changes as below: **Dependencies** - Replaced `hbase-shaded-jetty` i.e. Jetty 9 with `hbase-shaded-jetty-12-plus-core` and `hbase-shaded-jetty-12-plus-ee8` i.e. Jetty 12 EE8 - Upgrade `servlet.api.version` to `4.0.1` - Upgrade `tomcat.jasper.version` to `9.0.108` **Code Adjustments** - Modified import statements to use `org.eclipse.jetty.ee8` etc. as per Jetty 12 - Replace `HandlerCollection` with `Handler.Sequence` - Refer https://jetty.org/docs/jetty/12/programming-guide/migration/11-to-12.html#api-changes-handler-sequence - Adjust request log handler. - Refer https://jetty.org/docs/jetty/12/programming-guide/migration/11-to-12.html#api-changes-handler-requestlog - Adjust error handlers' set logic based on available methods - Remove usage of deprecated method `setResourceBase()` with `setBaseResourceAsPath()`. Also, passing `logDir` path in canonical format as otherwise, we get `404` in case we have `\..\` in static path. - Refer jetty/jetty.project#12958 - Remove `MultiException` with `ExceptionUtil.MultiException` in `HttpServer.java`. - Modify assignment: `webServer.getHandlers()` with new return type: `Array` to `List` - Replaced `Constraint` with `ServletConstraint` - Added `LogLevelExceptionUtils.java` to handle HTML error response for log levels as `connection.getResponseMessage()` now returns error code as string in message and hence causes test failure otherwise - Allow `javax` as xsd files at `javax/servlet/resources` come via `org.apache.hbase.thirdparty:hbase-shaded-jetty-12-ee8 ` **Test Adjustments** - Update `checkBindAddress()` to call `server.start()` else `testBindAddress()` fails with NPE. Verified doing same without Jetty 12 works as well. - Replace `JSON.parse()` with `new JSON().fromJSON()` and `JSON.toString()` with `new JSON().toJSON()` - Update app name from `".."` to `""` as it fails with `IllegalArgumentException` otherwise - Update `MockHttpApiRule` where we replace `AbstractHandler` with `Handler.Abstract` and adjust code based on new interface. - Refer https://jetty.org/docs/jetty/12/programming-guide/migration/11-to-12.html#handler-request-content-apis - Deleted `webdefaults.xml`. Need to see if still needed. - Added necessary compliance rules for URI validation, required by tests and possibly users as well. Add note for what fails if these rules are not there in tests. - Refer jetty/jetty.project#11890 (comment) **References** - Umbrealla at HBASE-29224 - Discussion at https://lists.apache.org/thread/bkrfm705kqd3bqzyvo7jv46t6p64x2n5 - Dependent thirdparty change at apache/hbase-thirdparty@fd76b1e Signed-off-by: Dávid Paksy <[email protected]> Signed-off-by: Istvan Toth <[email protected]>
…t to 4.0.1 (apache#6783)" This reverts commit 8bb4e8c.
Please find a detailed summary of all changes as below:
Dependencies
hbase-shaded-jettyi.e. Jetty 9 withhbase-shaded-jetty-12-plus-coreandhbase-shaded-jetty-12-plus-ee8i.e. Jetty 12 EE8servlet.api.versionto4.0.1tomcat.jasper.versionto9.0.107Code Adjustments
org.eclipse.jetty.ee8etc. as per Jetty 12HandlerCollectionwithHandler.SequencesetResourceBase()withsetBaseResourceAsPath(). Also, passinglogDirpath in canonical format as otherwise, we get404in case we have\..\in static path...jetty/jetty.project#12958MultiExceptionwithExceptionUtil.MultiExceptioninHttpServer.java.webServer.getHandlers()with new return type:ArraytoListConstraintwithServletConstraintLogLevelExceptionUtils.javato handle HTML error response for log levels asconnection.getResponseMessage()now returns error code as string in message and hence causes test failure otherwisejavaxas xsd files atjavax/servlet/resourcescome viaorg.apache.hbase.thirdparty:hbase-shaded-jetty-12-ee8Test Adjustments
checkBindAddress()to callserver.start()elsetestBindAddress()fails with NPE. Verified doing same without Jetty 12 works as well.JSON.parse()withnew JSON().fromJSON()andJSON.toString()withnew JSON().toJSON()".."to""as it fails withIllegalArgumentExceptionotherwiseMockHttpApiRulewhere we replaceAbstractHandlerwithHandler.Abstractand adjust code based on new interface.webdefaults.xml. Need to see if still needed.References