From a653366185f39bf01ded2ad4033993268116779c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 7 Aug 2023 20:16:24 +0000 Subject: [PATCH 01/31] Bump jmh.version from 1.36 to 1.37 Bumps `jmh.version` from 1.36 to 1.37. Updates `org.openjdk.jmh:jmh-core` from 1.36 to 1.37 - [Commits](https://github.com/openjdk/jmh/compare/1.36...1.37) Updates `org.openjdk.jmh:jmh-generator-annprocess` from 1.36 to 1.37 - [Commits](https://github.com/openjdk/jmh/compare/1.36...1.37) --- updated-dependencies: - dependency-name: org.openjdk.jmh:jmh-core dependency-type: direct:production update-type: version-update:semver-minor - dependency-name: org.openjdk.jmh:jmh-generator-annprocess dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index c35ade75de6..a98bdd612c4 100644 --- a/pom.xml +++ b/pom.xml @@ -88,7 +88,7 @@ 4.0.6 1.2 6.0 - 1.36 + 1.37 5.12.1 0.10.4 0.32.13 From 80cb8c2e89a08a54375cf4b36ff8dad9b8611dab Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 7 Aug 2023 20:27:55 +0000 Subject: [PATCH 02/31] Bump org.jboss.logmanager:jboss-logmanager Bumps [org.jboss.logmanager:jboss-logmanager](https://github.com/jboss-logging/jboss-logmanager) from 2.1.19.Final to 3.0.1.Final. - [Release notes](https://github.com/jboss-logging/jboss-logmanager/releases) - [Commits](https://github.com/jboss-logging/jboss-logmanager/compare/2.1.19.Final...3.0.1.Final) --- updated-dependencies: - dependency-name: org.jboss.logmanager:jboss-logmanager dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 2ffd01c2589..bd4b507e491 100644 --- a/pom.xml +++ b/pom.xml @@ -81,7 +81,7 @@ 2.2.1.Final 2.2.1.Final 3.5.3.Final - 2.1.19.Final + 3.0.1.Final 3.5.0.Final 1.1 1.0.7 From bf822a804689c5a07f423d3f5207c7f133670365 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 7 Aug 2023 22:54:49 +0000 Subject: [PATCH 03/31] Bump org.eclipse.cbi.maven.plugins:eclipse-jarsigner-plugin Bumps org.eclipse.cbi.maven.plugins:eclipse-jarsigner-plugin from 1.1.7 to 1.4.2. --- updated-dependencies: - dependency-name: org.eclipse.cbi.maven.plugins:eclipse-jarsigner-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a98bdd612c4..7cd12db7bcf 100644 --- a/pom.xml +++ b/pom.xml @@ -2378,7 +2378,7 @@ - 1.1.7 + 1.4.2 From e88835fcc78b9bf50a4cd0463e5da80b97626c40 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Aug 2023 00:51:14 +0000 Subject: [PATCH 04/31] Bump ch.qos.logback:logback-core from 1.3.8 to 1.3.9 Bumps [ch.qos.logback:logback-core](https://github.com/qos-ch/logback) from 1.3.8 to 1.3.9. - [Commits](https://github.com/qos-ch/logback/compare/v_1.3.8...v_1.3.9) --- updated-dependencies: - dependency-name: ch.qos.logback:logback-core dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7cd12db7bcf..b19da4d6000 100644 --- a/pom.xml +++ b/pom.xml @@ -102,7 +102,7 @@ 5.9.1 2.0.3 2.20.0 - 1.3.8 + 1.3.9 3.0.8 10.3.6 1.9.14 From 8d747a170c5d6e9142fb8619a2463858996de26c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Aug 2023 02:36:51 +0000 Subject: [PATCH 05/31] Bump maven.resolver.version from 1.9.14 to 1.9.15 Bumps `maven.resolver.version` from 1.9.14 to 1.9.15. Updates `org.apache.maven.resolver:maven-resolver-api` from 1.9.14 to 1.9.15 - [Release notes](https://github.com/apache/maven-resolver/releases) - [Commits](https://github.com/apache/maven-resolver/compare/maven-resolver-1.9.14...maven-resolver-1.9.15) Updates `org.apache.maven.resolver:maven-resolver-util` from 1.9.14 to 1.9.15 - [Release notes](https://github.com/apache/maven-resolver/releases) - [Commits](https://github.com/apache/maven-resolver/compare/maven-resolver-1.9.14...maven-resolver-1.9.15) Updates `org.apache.maven.resolver:maven-resolver-spi` from 1.9.14 to 1.9.15 - [Release notes](https://github.com/apache/maven-resolver/releases) - [Commits](https://github.com/apache/maven-resolver/compare/maven-resolver-1.9.14...maven-resolver-1.9.15) Updates `org.apache.maven.resolver:maven-resolver-impl` from 1.9.14 to 1.9.15 - [Release notes](https://github.com/apache/maven-resolver/releases) - [Commits](https://github.com/apache/maven-resolver/compare/maven-resolver-1.9.14...maven-resolver-1.9.15) Updates `org.apache.maven.resolver:maven-resolver-connector-basic` from 1.9.14 to 1.9.15 - [Release notes](https://github.com/apache/maven-resolver/releases) - [Commits](https://github.com/apache/maven-resolver/compare/maven-resolver-1.9.14...maven-resolver-1.9.15) Updates `org.apache.maven.resolver:maven-resolver-transport-file` from 1.9.14 to 1.9.15 - [Release notes](https://github.com/apache/maven-resolver/releases) - [Commits](https://github.com/apache/maven-resolver/compare/maven-resolver-1.9.14...maven-resolver-1.9.15) Updates `org.apache.maven.resolver:maven-resolver-transport-http` from 1.9.14 to 1.9.15 - [Release notes](https://github.com/apache/maven-resolver/releases) - [Commits](https://github.com/apache/maven-resolver/compare/maven-resolver-1.9.14...maven-resolver-1.9.15) --- updated-dependencies: - dependency-name: org.apache.maven.resolver:maven-resolver-api dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.apache.maven.resolver:maven-resolver-util dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.apache.maven.resolver:maven-resolver-spi dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.apache.maven.resolver:maven-resolver-impl dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.apache.maven.resolver:maven-resolver-connector-basic dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.apache.maven.resolver:maven-resolver-transport-file dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.apache.maven.resolver:maven-resolver-transport-http dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index b19da4d6000..0a3a8b9f254 100644 --- a/pom.xml +++ b/pom.xml @@ -105,7 +105,7 @@ 1.3.9 3.0.8 10.3.6 - 1.9.14 + 1.9.15 3.9.0 3.12.11 0.9.1 From 95981c2fbe052c2787abbf95df75bf9db57c36ee Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Aug 2023 04:11:17 +0000 Subject: [PATCH 06/31] Bump org.apache.maven.scm:maven-scm-provider-jgit from 2.0.0 to 2.0.1 Bumps org.apache.maven.scm:maven-scm-provider-jgit from 2.0.0 to 2.0.1. --- updated-dependencies: - dependency-name: org.apache.maven.scm:maven-scm-provider-jgit dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- jetty-util/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-util/pom.xml b/jetty-util/pom.xml index 5c538e628e0..53105e27c8b 100644 --- a/jetty-util/pom.xml +++ b/jetty-util/pom.xml @@ -44,7 +44,7 @@ org.apache.maven.scm maven-scm-provider-jgit - 2.0.0 + 2.0.1 From 04f793048a128e526aba1e6875cb7e29465ffa09 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Tue, 8 Aug 2023 15:11:51 +1000 Subject: [PATCH 07/31] upgrade buildnumber version as well Signed-off-by: Olivier Lamy --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 0a3a8b9f254..79894f52015 100644 --- a/pom.xml +++ b/pom.xml @@ -129,7 +129,7 @@ 2.2.4 3.3.0 - 3.0.0 + 3.2.0 1.5.0 1.3.0 1.0.10 From f424328c86c1ade9a3817c649400ef957b7df91a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Aug 2023 07:00:19 +0000 Subject: [PATCH 08/31] Bump org.jboss.logmanager:jboss-logmanager Bumps [org.jboss.logmanager:jboss-logmanager](https://github.com/jboss-logging/jboss-logmanager) from 2.1.19.Final to 3.0.1.Final. - [Release notes](https://github.com/jboss-logging/jboss-logmanager/releases) - [Commits](https://github.com/jboss-logging/jboss-logmanager/compare/2.1.19.Final...3.0.1.Final) --- updated-dependencies: - dependency-name: org.jboss.logmanager:jboss-logmanager dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 79894f52015..250c8ccb557 100644 --- a/pom.xml +++ b/pom.xml @@ -80,7 +80,7 @@ 2.2.1.Final 2.2.1.Final 3.5.3.Final - 2.1.19.Final + 3.0.1.Final 3.5.0.Final 1.1 1.0.7 From b5ef4efc5367e6047d4c3d7431c635de38b336a7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Aug 2023 07:05:12 +0000 Subject: [PATCH 09/31] Bump ch.qos.logback:logback-core from 1.4.8 to 1.4.9 Bumps [ch.qos.logback:logback-core](https://github.com/qos-ch/logback) from 1.4.8 to 1.4.9. - [Commits](https://github.com/qos-ch/logback/compare/v_1.4.8...v_1.4.9) --- updated-dependencies: - dependency-name: ch.qos.logback:logback-core dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index fbf76299aa1..972b2d99225 100644 --- a/pom.xml +++ b/pom.xml @@ -102,7 +102,7 @@ 5.9.1 2.0.3 2.20.0 - 1.4.8 + 1.4.9 3.0.8 10.3.6 1.9.15 From 665b2879c90635e03ed1fd2ea26252f6693ce6c2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 9 Aug 2023 22:42:17 +0000 Subject: [PATCH 10/31] Bump ch.qos.logback:logback-core from 1.3.9 to 1.3.11 Bumps [ch.qos.logback:logback-core](https://github.com/qos-ch/logback) from 1.3.9 to 1.3.11. - [Commits](https://github.com/qos-ch/logback/compare/v_1.3.9...v_1.3.11) --- updated-dependencies: - dependency-name: ch.qos.logback:logback-core dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 250c8ccb557..f7f3fa00690 100644 --- a/pom.xml +++ b/pom.xml @@ -102,7 +102,7 @@ 5.9.1 2.0.3 2.20.0 - 1.3.9 + 1.3.11 3.0.8 10.3.6 1.9.15 From 883a11464dbac8320943dac55375a2cdbd9cb41b Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 11 Aug 2023 09:54:20 +0200 Subject: [PATCH 11/31] Add Jetty Environment to issue-template.md --- .github/ISSUE_TEMPLATE/issue-template.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/issue-template.md b/.github/ISSUE_TEMPLATE/issue-template.md index 4ea70faa974..98e009121fc 100644 --- a/.github/ISSUE_TEMPLATE/issue-template.md +++ b/.github/ISSUE_TEMPLATE/issue-template.md @@ -10,6 +10,9 @@ labels: Bug **Jetty version(s)** +**Jetty Environment** + + **Java version/vendor** `(use: java -version)` **OS type/version** From 48955d3e8fea7b1723efa77a268f62b88621792d Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 11 Aug 2023 09:56:53 +0200 Subject: [PATCH 12/31] Update question-template.md --- .github/ISSUE_TEMPLATE/question-template.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/question-template.md b/.github/ISSUE_TEMPLATE/question-template.md index d9cb028ca8b..a36ca9e1bbe 100644 --- a/.github/ISSUE_TEMPLATE/question-template.md +++ b/.github/ISSUE_TEMPLATE/question-template.md @@ -7,9 +7,12 @@ assignees: '' --- -**Jetty version** +**Jetty Version** -**Java version** +**Jetty Environment** + + +**Java Version** **Question** From 812b8c473949dec357fd3f68e8eced5fc962e8f0 Mon Sep 17 00:00:00 2001 From: Jesse McConnell Date: Fri, 11 Aug 2023 11:38:10 -0500 Subject: [PATCH 13/31] Update release-template.md Add a link to jetty-website repo with instructions on deploying eclipse jetty website for new releases. --- .github/ISSUE_TEMPLATE/release-template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/release-template.md b/.github/ISSUE_TEMPLATE/release-template.md index feb328263f6..29e0a76e164 100644 --- a/.github/ISSUE_TEMPLATE/release-template.md +++ b/.github/ISSUE_TEMPLATE/release-template.md @@ -55,7 +55,7 @@ This release process will produce releases: - [ ] Promote staged releases. - [ ] Merge release branches back to main branches and delete release branches. - [ ] Verify release existence in Maven Central by triggering the Jenkins builds of CometD. -- [ ] Update Jetty versions on the web sites. +- [ ] Update Jetty versions on the website ( follow instructions in [jetty-website](https://github.com/eclipse/jetty-website/blob/master/README.md) ). + [ ] Update (or check) [Download](https://eclipse.dev/jetty/download.php) page is updated. + [ ] Update (or check) documentation page(s) are updated. - [ ] Publish GitHub Releases in the order of oldest (eg: 9) to newest (eg: 11) (to ensure that "latest" in github is truly the latest) From 98d25a788a9ebc55ea2831d6fa6be28bf564aed4 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Tue, 15 Aug 2023 15:11:26 +1000 Subject: [PATCH 14/31] Update Jenkinsfile-autobahn to not use not anymore available container (#10001) --- Jenkinsfile-autobahn | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/Jenkinsfile-autobahn b/Jenkinsfile-autobahn index eee85b9bbbe..3366e2d2ec0 100644 --- a/Jenkinsfile-autobahn +++ b/Jenkinsfile-autobahn @@ -26,17 +26,15 @@ pipeline { node { label 'linux' } } steps { - container( 'jetty-build' ) { - timeout( time: 120, unit: 'MINUTES' ) { - mavenBuild( "jdk11", "-T3 clean install -Djacoco.skip=true -pl :test-websocket-autobahn -am -Pautobahn -Dtest=AutobahnTests", "maven3" ) // - junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml,**/target/autobahntestsuite-reports/*.xml' - publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/core/servers", reportFiles: 'index.html', reportName: 'Autobahn Report Core Server', reportTitles: '']) - publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/core/clients", reportFiles: 'index.html', reportName: 'Autobahn Report Core Client', reportTitles: '']) - publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/javax/servers", reportFiles: 'index.html', reportName: 'Autobahn Report Javax Server', reportTitles: '']) - publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/javax/clients", reportFiles: 'index.html', reportName: 'Autobahn Report Javax Client', reportTitles: '']) - publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/jetty/servers", reportFiles: 'index.html', reportName: 'Autobahn Report Jetty Server', reportTitles: '']) - publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/jetty/clients", reportFiles: 'index.html', reportName: 'Autobahn Report Jetty Client', reportTitles: '']) - } + timeout( time: 120, unit: 'MINUTES' ) { + mavenBuild( "jdk11", "-T3 clean install -Djacoco.skip=true -pl :test-websocket-autobahn -am -Pautobahn -Dtest=AutobahnTests", "maven3" ) // + junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml,**/target/autobahntestsuite-reports/*.xml' + publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/core/servers", reportFiles: 'index.html', reportName: 'Autobahn Report Core Server', reportTitles: '']) + publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/core/clients", reportFiles: 'index.html', reportName: 'Autobahn Report Core Client', reportTitles: '']) + publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/javax/servers", reportFiles: 'index.html', reportName: 'Autobahn Report Javax Server', reportTitles: '']) + publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/javax/clients", reportFiles: 'index.html', reportName: 'Autobahn Report Javax Client', reportTitles: '']) + publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/jetty/servers", reportFiles: 'index.html', reportName: 'Autobahn Report Jetty Server', reportTitles: '']) + publishHTML([allowMissing: true, alwaysLinkToLastBuild: true, keepAll: true, reportDir: "${env.WORKSPACE}/tests/test-websocket-autobahn/target/reports/jetty/clients", reportFiles: 'index.html', reportName: 'Autobahn Report Jetty Client', reportTitles: '']) } } } From 42443c19c2261abfa9084f410c57b04dad5a5e2a Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 15 Aug 2023 11:34:16 -0500 Subject: [PATCH 15/31] Cleaning up dependencies on websocket to avoid pulling in annotations on websocket-server Signed-off-by: Joakim Erdfelt --- jetty-websocket/websocket-jetty-server/pom.xml | 2 +- jetty-websocket/websocket-jetty-tests/pom.xml | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/jetty-websocket/websocket-jetty-server/pom.xml b/jetty-websocket/websocket-jetty-server/pom.xml index d984adf0b3c..e31b9bbb7e1 100644 --- a/jetty-websocket/websocket-jetty-server/pom.xml +++ b/jetty-websocket/websocket-jetty-server/pom.xml @@ -37,7 +37,7 @@ org.eclipse.jetty - jetty-annotations + jetty-webapp org.eclipse.jetty diff --git a/jetty-websocket/websocket-jetty-tests/pom.xml b/jetty-websocket/websocket-jetty-tests/pom.xml index aa6d3ab6610..b3a7b26ee60 100644 --- a/jetty-websocket/websocket-jetty-tests/pom.xml +++ b/jetty-websocket/websocket-jetty-tests/pom.xml @@ -72,6 +72,11 @@ jetty-jmx test + + org.eclipse.jetty + jetty-annotations + test + From 00e01829cf95bd502cd5fd42581b4293ba27252c Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 15 Aug 2023 16:14:21 -0500 Subject: [PATCH 16/31] Bump guava to 32.1.2-jre Signed-off-by: Joakim Erdfelt --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f7f3fa00690..28ff2e7b631 100644 --- a/pom.xml +++ b/pom.xml @@ -47,7 +47,7 @@ 2.15.0 1.49.2 2.9.1 - 31.1-jre + 32.1.2-jre 5.1.0 2.2 2.15.2 From b126407c4eaaa4c24101c2bcf6e406f47d196038 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 16 Aug 2023 13:13:41 +0200 Subject: [PATCH 17/31] Fixed Maven coordinates, wrongly reported just as `jetty-servlet`, to `jetty-eeN-servlet`. Signed-off-by: Simone Bordet --- .../server/http/server-http-handler-use.adoc | 40 +++++++++++++++++-- .../server/http/HTTPServerDocs.java | 25 +++++++----- 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/documentation/jetty-documentation/src/main/asciidoc/programming-guide/server/http/server-http-handler-use.adoc b/documentation/jetty-documentation/src/main/asciidoc/programming-guide/server/http/server-http-handler-use.adoc index 5abb2c3aeaf..1dea2d3b092 100644 --- a/documentation/jetty-documentation/src/main/asciidoc/programming-guide/server/http/server-http-handler-use.adoc +++ b/documentation/jetty-documentation/src/main/asciidoc/programming-guide/server/http/server-http-handler-use.adoc @@ -325,20 +325,28 @@ Jetty will send an HTTP `404` response anyway if `DefaultHandler` is not used. `ServletContextHandler` is a `ContextHandler` that provides support for the Servlet APIs and implements the behaviors required by the Servlet specification. -The Maven artifact coordinates are: +The Maven artifact coordinates depend on the version of Jakarta EE you want to use, and they are: [source,xml,subs=normal] ---- - org.eclipse.jetty - jetty-servlet + org.eclipse.jetty.{ee-all} + jetty-{ee-all}-servlet {version} ---- +For example, for Jakarta {ee-current-caps} the coordinates are: `org.eclipse.jetty.ee10:jetty-ee10-servlet:{version}`. + +Below you can find an example of how to setup a Jakarta {ee-current-caps} `ServletContextHandler`: + [source,java,indent=0] ---- -include::../../{doc_code}/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java[tags=servletContextHandler] +include::../../{doc_code}/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java[tags=servletContextHandler-servlet] +---- +[source,java,indent=0] +---- +include::../../{doc_code}/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java[tags=servletContextHandler-setup] ---- The `Handler` and Servlet components tree structure looks like the following: @@ -368,6 +376,17 @@ Server applications must be careful when creating the `Handler` tree to put ``Se `WebAppContext` is a `ServletContextHandler` that auto configures itself by reading a `web.xml` Servlet configuration file. +The Maven artifact coordinates depend on the version of Jakarta EE you want to use, and they are: + +[source,xml,subs=normal] +---- + + org.eclipse.jetty.{ee-all} + jetty-{ee-all}-webapp + {version} + +---- + Server applications can specify a `+*.war+` file or a directory with the structure of a `+*.war+` file to `WebAppContext` to deploy a standard Servlet web application packaged as a `war` (as defined by the Servlet specification). Where server applications using `ServletContextHandler` must manually invoke methods to add ``Servlet``s and ``Filter``s, `WebAppContext` reads `WEB-INF/web.xml` to add ``Servlet``s and ``Filter``s, and also enforces a number of restrictions defined by the Servlet specification, in particular related to class loading. @@ -412,6 +431,19 @@ However, Jetty picks good defaults and allows server applications to customize _ If you have a xref:pg-server-http-handler-use-servlet-context[Servlet web application], you may want to use a `DefaultServlet` instead of `ResourceHandler`. The features are similar, but `DefaultServlet` is more commonly used to serve static files for Servlet web applications. +The Maven artifact coordinates depend on the version of Jakarta EE you want to use, and they are: + +[source,xml,subs=normal] +---- + + org.eclipse.jetty.{ee-all} + jetty-{ee-all}-servlet + {version} + +---- + +Below you can find an example of how to setup `DefaultServlet`: + [source,java,indent=0] ---- include::../../{doc_code}/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java[tags=defaultServlet] diff --git a/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java b/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java index 07a0cdcc664..347e15221d4 100644 --- a/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java +++ b/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java @@ -658,18 +658,21 @@ public class HTTPServerDocs // end::contextHandlerCollection[] } + @SuppressWarnings("InnerClassMayBeStatic") + // tag::servletContextHandler-servlet[] + public class ShopCartServlet extends HttpServlet + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse response) + { + // Implement the shop cart functionality. + } + } + // end::servletContextHandler-servlet[] + public void servletContextHandler() throws Exception { - // tag::servletContextHandler[] - class ShopCartServlet extends HttpServlet - { - @Override - protected void service(HttpServletRequest request, HttpServletResponse response) - { - // Implement the shop cart functionality. - } - } - + // tag::servletContextHandler-setup[] Server server = new Server(); Connector connector = new ServerConnector(server); server.addConnector(connector); @@ -692,7 +695,7 @@ public class HTTPServerDocs server.setHandler(context); server.start(); - // end::servletContextHandler[] + // end::servletContextHandler-setup[] } public void webAppContextHandler() throws Exception From 8f6a38aca8eef02e221ae73307667816906a91cb Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 11 Aug 2023 19:10:55 +0200 Subject: [PATCH 18/31] Added documentation to migrate from Servlet to Handler. Introduced CompletableTask to simplify Content.Source reads. Signed-off-by: Simone Bordet --- .../migration/migration-11-to-12.adoc | 44 ++ .../migration/ServletToHandlerDocs.java | 643 ++++++++++++++++++ .../eclipse/jetty/io/ContentSourceTest.java | 33 +- .../eclipse/jetty/util/CompletableTask.java | 76 +++ 4 files changed, 777 insertions(+), 19 deletions(-) create mode 100644 documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java create mode 100644 jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/CompletableTask.java diff --git a/documentation/jetty-documentation/src/main/asciidoc/programming-guide/migration/migration-11-to-12.adoc b/documentation/jetty-documentation/src/main/asciidoc/programming-guide/migration/migration-11-to-12.adoc index 325b03dcf22..32972a382ad 100644 --- a/documentation/jetty-documentation/src/main/asciidoc/programming-guide/migration/migration-11-to-12.adoc +++ b/documentation/jetty-documentation/src/main/asciidoc/programming-guide/migration/migration-11-to-12.adoc @@ -97,8 +97,52 @@ [[pg-migration-11-to-12-servlet-to-handler]] ==== Migrate Servlets to Jetty Handlers +Web applications written using the Servlet APIs may be re-written using the Jetty `Handler` APIs. +The sections below outline the Jetty `Handler` APIs that correspond to the Servlet APIs. +To replace the functionalities of Servlet Filters, refer to xref:pg-server-http-handler[this section]. +===== Handler Request APIs +[source,java,indent=0] +---- +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=request] +---- + +===== Handler Request Content APIs +[source,java,indent=0] +---- +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=requestContent-string] + +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=requestContent-buffer] + +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=requestContent-stream] + +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=requestContent-source] +---- + +===== Handler Response APIs +[source,java,indent=0] +---- +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=response] +---- + +===== Handler Response Content APIs +[source,java,indent=0] +---- +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=responseContent-implicit] + +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=responseContent-implicit-status] + +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=responseContent-explicit] + +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=responseContent-content] + +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=responseContent-string] + +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=responseContent-echo] + +include::../{doc_code}/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java[tags=responseContent-trailers] +---- [[pg-migration-11-to-12-api-changes]] ==== APIs Changes diff --git a/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java b/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java new file mode 100644 index 00000000000..4e71ee37907 --- /dev/null +++ b/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/migration/ServletToHandlerDocs.java @@ -0,0 +1,643 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.docs.programming.migration; + +import java.io.InputStream; +import java.nio.ByteBuffer; +import java.time.Duration; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.function.Supplier; + +import org.eclipse.jetty.http.HttpCookie; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.Trailers; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Context; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.server.Session; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.CompletableTask; +import org.eclipse.jetty.util.Fields; +import org.eclipse.jetty.util.Promise; + +import static java.nio.charset.StandardCharsets.UTF_8; + +@SuppressWarnings("unused") +public class ServletToHandlerDocs +{ + @SuppressWarnings("InnerClassMayBeStatic") + // tag::request[] + public class RequestAPIs extends Handler.Abstract + { + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + // Gets the request method. + // Replaces: + // - servletRequest.getMethod(); + String method = request.getMethod(); + + // Gets the request protocol name and version. + // Replaces: + // - servletRequest.getProtocol(); + String protocol = request.getConnectionMetaData().getProtocol(); + + // Gets the full request URI. + // Replaces: + // - servletRequest.getRequestURL(); + String fullRequestURI = request.getHttpURI().asString(); + + // Gets the request context. + // Replaces: + // - servletRequest.getServletContext() + Context context = request.getContext(); + + // Gets the context path. + // Replaces: + // - servletRequest.getContextPath() + String contextPath = context.getContextPath(); + + // Gets the request path. + // Replaces: + // - servletRequest.getRequestURI(); + String requestPath = request.getHttpURI().getPath(); + + // Gets the request path after the context path. + // Replaces: + // - servletRequest.getServletPath() + servletRequest.getPathInfo() + String pathInContext = Request.getPathInContext(request); + + // Gets the request query. + // Replaces: + // - servletRequest.getQueryString() + String queryString = request.getHttpURI().getQuery(); + + // Gets request parameters. + // Replaces: + // - servletRequest.getParameterNames(); + // - servletRequest.getParameter(name); + // - servletRequest.getParameterValues(name); + // - servletRequest.getParameterMap(); + Fields queryParameters = Request.extractQueryParameters(request, UTF_8); + Fields allParameters = Request.getParameters(request); + + // Gets cookies. + // Replaces: + // - servletRequest.getCookies(); + List cookies = Request.getCookies(request); + + // Gets request HTTP headers. + // Replaces: + // - servletRequest.getHeaderNames() + // - servletRequest.getHeader(name) + // - servletRequest.getHeaders(name) + // - servletRequest.getDateHeader(name) + // - servletRequest.getIntHeader(name) + HttpFields requestHeaders = request.getHeaders(); + + // Gets the request Content-Type. + // Replaces: + // - servletRequest.getContentType() + String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE); + + // Gets the request Content-Length. + // Replaces: + // - servletRequest.getContentLength() + // - servletRequest.getContentLengthLong() + long contentLength = request.getHeaders().getLongField(HttpHeader.CONTENT_LENGTH); + + // Gets the request locales. + // Replaces: + // - servletRequest.getLocale() + // - servletRequest.getLocales() + List locales = Request.getLocales(request); + + // Gets the request scheme. + // Replaces: + // - servletRequest.getScheme() + String scheme = request.getHttpURI().getScheme(); + + // Gets the server name. + // Replaces: + // - servletRequest.getServerName() + String serverName = Request.getServerName(request); + + // Gets the server port. + // Replaces: + // - servletRequest.getServerPort() + int serverPort = Request.getServerPort(request); + + // Gets the remote host/address. + // Replaces: + // - servletRequest.getRemoteAddr() + // - servletRequest.getRemoteHost() + String remoteAddress = Request.getRemoteAddr(request); + + // Gets the remote port. + // Replaces: + // - servletRequest.getRemotePort() + int remotePort = Request.getRemotePort(request); + + // Gets the local host/address. + // Replaces: + // - servletRequest.getLocalAddr() + // - servletRequest.getLocalHost() + String localAddress = Request.getLocalAddr(request); + + // Gets the local port. + // Replaces: + // - servletRequest.getLocalPort() + int localPort = Request.getLocalPort(request); + + // Gets the request attributes. + // Replaces: + // - servletRequest.getAttributeNames() + // - servletRequest.getAttribute(name) + // - servletRequest.setAttribute(name, value) + // - servletRequest.removeAttribute(name) + String name = "name"; + Object value = "value"; + Set names = request.getAttributeNameSet(); + Object attribute = request.getAttribute(name); + request.setAttribute(name, value); + request.removeAttribute(name); + request.clearAttributes(); + Map map = request.asAttributeMap(); + + // Gets the request trailers. + // Replaces: + // - servletRequest.getTrailerFields() + HttpFields trailers = request.getTrailers(); + + // Gets the HTTP session. + // Replaces: + // - servletRequest.getSession() + // - servletRequest.getSession(create) + boolean create = true; + Session session = request.getSession(create); + + callback.succeeded(); + return false; + } + } + // end::request[] + + @SuppressWarnings("InnerClassMayBeStatic") + public class RequestContentAPIsString extends Handler.Abstract + { + // tag::requestContent-string[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + // Non-blocking read the request content as a String. + // Use with caution as the request content may be large. + Promise.Completable completable = new Promise.Completable<>(); + Content.Source.asString(request, UTF_8, completable); + + completable.whenComplete((requestContent, failure) -> + { + if (failure == null) + { + // Process the request content here. + + // Implicitly respond with status code 200 and no content. + callback.succeeded(); + } + else + { + // Implicitly respond with status code 500. + callback.failed(failure); + } + }); + + return true; + } + // end::requestContent-string[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + public class RequestContentAPIsByteBuffer extends Handler.Abstract + { + // tag::requestContent-buffer[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + // Non-blocking read the request content as a ByteBuffer. + // Use with caution as the request content may be large. + Promise.Completable completable = new Promise.Completable<>(); + Content.Source.asByteBuffer(request, completable); + + completable.whenComplete((requestContent, failure) -> + { + if (failure == null) + { + // Process the request content here. + + // Implicitly respond with status code 200 and no content. + callback.succeeded(); + } + else + { + // Implicitly respond with status code 500. + callback.failed(failure); + } + }); + + return true; + } + // end::requestContent-buffer[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + public class RequestContentAPIsInputStream extends Handler.Abstract + { + // tag::requestContent-stream[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + // Read the request content as an InputStream. + // Note that InputStream.read() may block. + try (InputStream inputStream = Content.Source.asInputStream(request)) + { + while (true) + { + int read = inputStream.read(); + + // EOF was reached, stop reading. + if (read < 0) + break; + + // Process the read byte here. + } + } + + // Implicitly respond with status code 200 and no content. + callback.succeeded(); + return true; + } + // end::requestContent-stream[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + public class RequestContentAPIsSource extends Handler.Abstract + { + // tag::requestContent-source[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + CompletableTask reader = new CompletableTask<>() + { + @Override + public void run() + { + // Read in a loop. + while (true) + { + // Read a chunk of content. + Content.Chunk chunk = request.read(); + + // If there is no content, demand to be + // called back when more content is available. + if (chunk == null) + { + request.demand(this); + return; + } + + // If a failure is read, complete with a failure. + if (Content.Chunk.isFailure(chunk)) + { + Throwable failure = chunk.getFailure(); + completeExceptionally(failure); + return; + } + + if (chunk instanceof Trailers trailers) + { + // Possibly process the request trailers here. + // Trailers have an empty ByteBuffer and are a last chunk. + } + + // Process the request content chunk here. + // After the processing, the chunk MUST be released. + chunk.release(); + + // If the last chunk is read, complete normally. + if (chunk.isLast()) + { + complete(null); + return; + } + + // Not the last chunk of content, loop around to read more. + } + } + }; + + // Initiate the read of the request content. + reader.start(); + + // When the read is complete, complete the Handler callback. + callback.completeWith(reader); + + return true; + } + // end::requestContent-source[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + // tag::response[] + public class ResponseAPIs extends Handler.Abstract + { + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + // Sets/Gets the response HTTP status. + // Replaces: + // - servletResponse.setStatus(code); + // - servletResponse.getStatus(); + response.setStatus(HttpStatus.OK_200); + int status = response.getStatus(); + + // Gets the response HTTP headers. + // Replaces: + // - servletResponse.setHeader(name, value); + // - servletResponse.addHeader(name, value); + // - servletResponse.setDateHeader(name, date); + // - servletResponse.addDateHeader(name, date); + // - servletResponse.setIntHeader(name, value); + // - servletResponse.addIntHeader(name, value); + // - servletResponse.getHeaderNames() + // - servletResponse.getHeader(name) + // - servletResponse.getHeaders(name) + // - servletResponse.containsHeader(name) + HttpFields.Mutable responseHeaders = response.getHeaders(); + + // Sets an HTTP cookie. + // Replaces: + // - servletResponse.addCookie(cookie); + HttpCookie cookie = HttpCookie.build("name", "value") + .domain("example.org") + .path("/path") + .maxAge(Duration.ofDays(1).toSeconds()) + .build(); + Response.addCookie(response, cookie); + + // Sets the response Content-Type. + // Replaces: + // - servletResponse.setContentType(type) + responseHeaders.put(HttpHeader.CONTENT_TYPE, "text/plain; charset=UTF-8"); + + // Sets the response Content-Length. + // Replaces: + // - servletResponse.setContentLength(length) + // - servletResponse.setContentLengthLong(length) + responseHeaders.put(HttpHeader.CONTENT_LENGTH, 1024L); + + // Sets/Gets the response trailers. + // Replaces: + // - servletResponse.setTrailerFields(() -> trailers) + // - servletResponse.getTrailerFields() + HttpFields trailers = HttpFields.build().put("checksum", 0xCAFE); + response.setTrailersSupplier(trailers); + Supplier trailersSupplier = response.getTrailersSupplier(); + + // Gets whether the response is committed. + // Replaces: + // - servletResponse.isCommitted() + boolean committed = response.isCommitted(); + + // Resets the response. + // Replaces: + // - servletResponse.reset(); + response.reset(); + + // Sends a redirect response. + // Replaces: + // - servletResponse.encodeRedirectURL(location) + // - servletResponse.sendRedirect(location) + String location = Request.toRedirectURI(request, "/redirect"); + Response.sendRedirect(request, response, callback, location); + + // Sends an error response. + // Replaces: + // - servletResponse.sendError(code); + // - servletResponse.sendError(code, message); + Response.writeError(request, response, callback, HttpStatus.SERVICE_UNAVAILABLE_503, "Request Cannot be Processed"); + + callback.succeeded(); + return true; + } + } + // end::response[] + + @SuppressWarnings("InnerClassMayBeStatic") + public class ResponseContentAPIsImplicit extends Handler.Abstract + { + // tag::responseContent-implicit[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + // Produces an implicit response with status code 200 + // with no content when returning from this method. + + // The Handler callback must be completed when returning true. + callback.succeeded(); + return true; + } + // end::responseContent-implicit[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + public class ResponseContentAPIsImplicitWithStatus extends Handler.Abstract + { + // tag::responseContent-implicit-status[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + // Produces an implicit response with status 204 + // with no content when returning from this method. + response.setStatus(HttpStatus.NO_CONTENT_204); + + // The Handler callback must be completed when returning true. + callback.succeeded(); + return true; + } + // end::responseContent-implicit-status[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + public class ResponseContentAPIsExplicit extends Handler.Abstract + { + // tag::responseContent-explicit[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + // Produces an explicit response with status 204 with no content. + response.setStatus(HttpStatus.NO_CONTENT_204); + + // This explicit first write() writes the response status code and headers. + // It is also the last write (as specified by the first parameter) + // and writes an empty content (the second parameter, a null ByteBuffer). + // When this write completes, the Handler callback is completed. + response.write(true, null, callback); + + return true; + } + // end::responseContent-explicit[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + public class ResponseContentAPISimpleContent extends Handler.Abstract + { + // tag::responseContent-content[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.setStatus(HttpStatus.OK_200); + + ByteBuffer content = UTF_8.encode("Hello World"); + + // Explicit first write that writes the response status code, headers and content. + // When this write completes, the Handler callback is completed. + response.write(true, content, callback); + + return true; + } + // end::responseContent-content[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + public class ResponseContentAPIFlush extends Handler.Abstract + { + // tag::responseContent-content[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.setStatus(HttpStatus.OK_200); + + ByteBuffer content = UTF_8.encode("Hello World"); + response.getHeaders().put(HttpHeader.CONTENT_LENGTH, content.remaining()); + + // Flush the response status code and the headers (no content). + // This is the fist but non-last write. + Callback.Completable completable = new Callback.Completable(); + response.write(false, null, completable); + + // When the first write completes, perform the second (and last) write. + completable.whenComplete((ignored, failure) -> + { + if (failure == null) + { + // Now explicitly write the content as the last write. + // When this write completes, the Handler callback is completed. + response.write(true, content, callback); + } + else + { + // Implicitly respond with status code 500. + callback.failed(failure); + } + }); + + return true; + } + // end::responseContent-content[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + public class ResponseContentAPIString extends Handler.Abstract + { + // tag::responseContent-string[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.setStatus(HttpStatus.OK_200); + + // Utility method to write UTF-8 string content. + // When this write completes, the Handler callback is completed. + Content.Sink.write(response, true, "Hello World", callback); + + return true; + } + // end::responseContent-string[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + public class ResponseContentAPIEcho extends Handler.Abstract + { + // tag::responseContent-echo[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.setStatus(HttpStatus.OK_200); + + // Utility method to echo the content from the request to the response. + // When the echo completes, the Handler callback is completed. + Content.copy(request, response, callback); + + return true; + } + // end::responseContent-echo[] + } + + @SuppressWarnings("InnerClassMayBeStatic") + public class ResponseContentAPITrailers extends Handler.Abstract + { + // tag::responseContent-trailers[] + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response.setStatus(HttpStatus.OK_200); + + // The trailers must be set on the response before the first write. + HttpFields.Mutable trailers = HttpFields.build(); + response.setTrailersSupplier(trailers); + + // Explicit first write that writes the response status code, headers and content. + // The trailers have not been written yet; they will be written with the last write. + ByteBuffer content = UTF_8.encode("Hello World"); + Callback.Completable completable = new Callback.Completable(); + response.write(false, content, completable); + + completable.whenComplete((ignored, failure) -> + { + if (failure == null) + { + // Update the trailers + trailers.put("Content-Checksum", 0xCAFE); + + // Explicit last write to write the trailers + // and complete the Handler callback. + response.write(true, null, callback); + } + else + { + // Implicitly respond with status code 500. + callback.failed(failure); + } + }); + + return true; + } + // end::responseContent-trailers[] + } +} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java index 1f7fd012420..7b1868d0fc5 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java @@ -42,6 +42,7 @@ import org.eclipse.jetty.io.content.PathContentSource; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.CompletableTask; import org.eclipse.jetty.util.FutureCallback; import org.eclipse.jetty.util.FuturePromise; import org.eclipse.jetty.util.IO; @@ -104,7 +105,9 @@ public class ContentSourceTest return List.of(asyncSource, byteBufferSource, transformerSource, pathSource, inputSource, inputSource2); } - /** Get the next chunk, blocking if necessary + /** + * Get the next chunk, blocking if necessary + * * @param source The source to get the next chunk from * @return A non null chunk */ @@ -113,8 +116,7 @@ public class ContentSourceTest Content.Chunk chunk = source.read(); if (chunk != null) return chunk; - FuturePromise next = new FuturePromise<>(); - Runnable getNext = new Runnable() + CompletableTask task = new CompletableTask<>() { @Override public void run() @@ -122,18 +124,12 @@ public class ContentSourceTest Content.Chunk chunk = source.read(); if (chunk == null) source.demand(this); - next.succeeded(chunk); + else + complete(chunk); } }; - source.demand(getNext); - try - { - return next.get(); - } - catch (Exception e) - { - throw new RuntimeException(e); - } + source.demand(task); + return task.join(); } @ParameterizedTest @@ -141,8 +137,7 @@ public class ContentSourceTest public void testRead(Content.Source source) throws Exception { StringBuilder builder = new StringBuilder(); - CountDownLatch eof = new CountDownLatch(1); - source.demand(new Runnable() + var task = new CompletableTask<>() { @Override public void run() @@ -162,14 +157,14 @@ public class ContentSourceTest if (chunk.isLast()) { - eof.countDown(); + complete(null); break; } } } - }); - - assertTrue(eof.await(10, TimeUnit.SECONDS)); + }; + source.demand(task); + task.get(10, TimeUnit.SECONDS); assertThat(builder.toString(), is("onetwo")); } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/CompletableTask.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/CompletableTask.java new file mode 100644 index 00000000000..4220698c96d --- /dev/null +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/CompletableTask.java @@ -0,0 +1,76 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.util; + +import java.util.concurrent.CompletableFuture; + +/** + *

A {@link CompletableFuture} that implements {@link Runnable} to perform + * a one-shot task that eventually completes this {@link CompletableFuture}.

+ *

Subclasses override {@link #run()} to implement the task.

+ *

Users of this class start the task execution via {@link #start()}.

+ *

Typical usage:

+ * 
{@code
+ * CompletableTask task = new CompletableTask<>()
+ * {
+ *     @Override
+ *     public void run()
+ *     {
+ *         try
+ *         {
+ *             // Perform some task.
+ *             T result = performTask();
+ *
+ *             // Eventually complete this CompletableFuture.
+ *             complete(result);
+ *         }
+ *         catch (Throwable x)
+ *         {
+ *             completeExceptionally(x);
+ *         }
+ *     }
+ * }
+ *
+ * // Start the task and then process the
+ * // result of the task when it is complete.
+ * task.start()
+ *     .whenComplete((result, failure) ->
+ *     {
+ *         if (failure == null)
+ *         {
+ *             // The task completed successfully.
+ *         }
+ *         else
+ *         {
+ *             // The task failed.
+ *         }
+ *     });
+ * }
+ * + * @param the type of the result of the task + */ +public abstract class CompletableTask extends CompletableFuture implements Runnable +{ + /** + *

Starts the task by calling {@link #run()} + * and returns this {@link CompletableTask}.

+ * + * @return this {@link CompletableTask} + */ + public CompletableTask start() + { + run(); + return this; + } +} From 17c36497714ed7d9ba5bb93ee82662c0f97fcbd3 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 16 Aug 2023 15:26:57 +0200 Subject: [PATCH 19/31] Fixes #10217 - Review ProxyConnectionFactory buffer management. (#10225) Fixed buffer leak in ProxyConnection classes. Introduced ArrayByteBufferPool.Tracking to test buffer leaks. Signed-off-by: Simone Bordet --- .../client/HttpClientProxyProtocolTest.java | 20 ++- .../test/resources/jetty-logging.properties | 1 + .../eclipse/jetty/io/ArrayByteBufferPool.java | 114 ++++++++++++++++++ .../jetty/server/ProxyConnectionFactory.java | 59 +++------ .../jetty/server/MultiPartByteRangesTest.java | 35 +----- 5 files changed, 153 insertions(+), 76 deletions(-) diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java index 89928e257df..30a995f3a3b 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java @@ -16,6 +16,7 @@ package org.eclipse.jetty.client; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.concurrent.ThreadLocalRandom; import org.eclipse.jetty.client.transport.HttpDestination; @@ -23,6 +24,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.io.ArrayByteBufferPool; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.server.Handler; @@ -33,6 +35,7 @@ import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -46,15 +49,18 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class HttpClientProxyProtocolTest { + private ArrayByteBufferPool.Tracking serverBufferPool; private Server server; private ServerConnector connector; + private ArrayByteBufferPool.Tracking clientBufferPool; private HttpClient client; private void startServer(Handler handler) throws Exception { QueuedThreadPool serverThreads = new QueuedThreadPool(); serverThreads.setName("server"); - server = new Server(serverThreads); + serverBufferPool = new ArrayByteBufferPool.Tracking(); + server = new Server(serverThreads, null, serverBufferPool); HttpConnectionFactory http = new HttpConnectionFactory(); ProxyConnectionFactory proxy = new ProxyConnectionFactory(http.getProtocol()); connector = new ServerConnector(server, 1, 1, proxy, http); @@ -67,18 +73,22 @@ public class HttpClientProxyProtocolTest { QueuedThreadPool clientThreads = new QueuedThreadPool(); clientThreads.setName("client"); + clientBufferPool = new ArrayByteBufferPool.Tracking(); client = new HttpClient(); client.setExecutor(clientThreads); + client.setByteBufferPool(clientBufferPool); client.start(); } @AfterEach public void dispose() throws Exception { - if (server != null) - server.stop(); - if (client != null) - client.stop(); + LifeCycle.stop(client); + LifeCycle.stop(server); + Set serverLeaks = serverBufferPool.getLeaks(); + assertEquals(0, serverLeaks.size(), serverBufferPool.dumpLeaks()); + Set clientLeaks = clientBufferPool.getLeaks(); + assertEquals(0, clientLeaks.size(), clientBufferPool.dumpLeaks()); } @Test diff --git a/jetty-core/jetty-client/src/test/resources/jetty-logging.properties b/jetty-core/jetty-client/src/test/resources/jetty-logging.properties index e194e90d87c..90423896e45 100644 --- a/jetty-core/jetty-client/src/test/resources/jetty-logging.properties +++ b/jetty-core/jetty-client/src/test/resources/jetty-logging.properties @@ -1,5 +1,6 @@ #org.eclipse.jetty.LEVEL=DEBUG #org.eclipse.jetty.client.LEVEL=DEBUG +#org.eclipse.jetty.io.ArrayByteBufferPool$Tracking.LEVEL=DEBUG #org.eclipse.jetty.io.SocketChannelEndPoint.LEVEL=DEBUG #org.eclipse.jetty.io.ssl.LEVEL=DEBUG #org.eclipse.jetty.http.LEVEL=DEBUG diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index ff914e0590d..2d906197bf9 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -14,11 +14,17 @@ package org.eclipse.jetty.io; import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.nio.ByteBuffer; +import java.time.Instant; import java.util.Arrays; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Consumer; import java.util.function.IntUnaryOperator; +import java.util.stream.Collectors; import org.eclipse.jetty.io.internal.CompoundPool; import org.eclipse.jetty.io.internal.QueuedPool; @@ -564,4 +570,112 @@ public class ArrayByteBufferPool implements ByteBufferPool, Dumpable ); } } + + /** + *

A variant of {@link ArrayByteBufferPool} that tracks buffer + * acquires/releases, useful to identify buffer leaks.

+ *

Use {@link #getLeaks()} when the system is idle to get + * the {@link Buffer}s that have been leaked, which contain + * the stack trace information of where the buffer was acquired.

+ */ + public static class Tracking extends ArrayByteBufferPool + { + private static final Logger LOG = LoggerFactory.getLogger(Tracking.class); + + private final Set buffers = ConcurrentHashMap.newKeySet(); + + public Tracking() + { + this(0, -1, Integer.MAX_VALUE); + } + + public Tracking(int minCapacity, int maxCapacity, int maxBucketSize) + { + this(minCapacity, maxCapacity, maxBucketSize, -1L, -1L); + } + + public Tracking(int minCapacity, int maxCapacity, int maxBucketSize, long maxHeapMemory, long maxDirectMemory) + { + super(minCapacity, -1, maxCapacity, maxBucketSize, maxHeapMemory, maxDirectMemory); + } + + @Override + public RetainableByteBuffer acquire(int size, boolean direct) + { + RetainableByteBuffer buffer = super.acquire(size, direct); + Buffer wrapper = new Buffer(buffer, size); + if (LOG.isDebugEnabled()) + LOG.debug("acquired {}", wrapper); + buffers.add(wrapper); + return wrapper; + } + + public Set getLeaks() + { + return buffers; + } + + public String dumpLeaks() + { + return getLeaks().stream() + .map(Buffer::dump) + .collect(Collectors.joining(System.lineSeparator())); + } + + public class Buffer extends RetainableByteBuffer.Wrapper + { + private final int size; + private final Instant acquireInstant; + private final Throwable acquireStack; + + private Buffer(RetainableByteBuffer wrapped, int size) + { + super(wrapped); + this.size = size; + this.acquireInstant = Instant.now(); + this.acquireStack = new Throwable(); + } + + public int getSize() + { + return size; + } + + public Instant getAcquireInstant() + { + return acquireInstant; + } + + public Throwable getAcquireStack() + { + return acquireStack; + } + + @Override + public boolean release() + { + boolean released = super.release(); + if (released) + { + buffers.remove(this); + if (LOG.isDebugEnabled()) + LOG.debug("released {}", this); + } + return released; + } + + public String dump() + { + StringWriter w = new StringWriter(); + getAcquireStack().printStackTrace(new PrintWriter(w)); + return "%s of %d bytes on %s at %s".formatted(getClass().getSimpleName(), getSize(), getAcquireInstant(), w); + } + + @Override + public String toString() + { + return "%s@%x[%s]".formatted(getClass().getSimpleName(), hashCode(), super.toString()); + } + } + } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java index d2cc7560728..548db964aa3 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java @@ -42,7 +42,7 @@ import org.slf4j.LoggerFactory; *

This factory can be placed in front of any other connection factory * to process the proxy v1 or v2 line before the normal protocol handling

* - * @see http://www.haproxy.org/download/1.5/doc/proxy-protocol.txt + * @see PROXY protocol */ public class ProxyConnectionFactory extends DetectorConnectionFactory { @@ -245,6 +245,7 @@ public class ProxyConnectionFactory extends DetectorConnectionFactory _buffer.release(); return unconsumed; } + _buffer.release(); return null; } @@ -564,6 +565,7 @@ public class ProxyConnectionFactory extends DetectorConnectionFactory _buffer.release(); return unconsumed; } + _buffer.release(); return null; } @@ -591,7 +593,7 @@ public class ProxyConnectionFactory extends DetectorConnectionFactory SocketAddress remote; switch (_family) { - case INET: + case INET -> { byte[] addr = new byte[4]; byteBuffer.get(addr); @@ -602,9 +604,8 @@ public class ProxyConnectionFactory extends DetectorConnectionFactory int dstPort = byteBuffer.getChar(); local = new InetSocketAddress(dstAddr, dstPort); remote = new InetSocketAddress(srcAddr, srcPort); - break; } - case INET6: + case INET6 -> { byte[] addr = new byte[16]; byteBuffer.get(addr); @@ -615,9 +616,8 @@ public class ProxyConnectionFactory extends DetectorConnectionFactory int dstPort = byteBuffer.getChar(); local = new InetSocketAddress(dstAddr, dstPort); remote = new InetSocketAddress(srcAddr, srcPort); - break; } - case UNIX: + case UNIX -> { byte[] addr = new byte[108]; byteBuffer.get(addr); @@ -626,12 +626,8 @@ public class ProxyConnectionFactory extends DetectorConnectionFactory String dst = UnixDomain.toPath(addr); local = UnixDomain.newSocketAddress(dst); remote = UnixDomain.newSocketAddress(src); - break; - } - default: - { - throw new IllegalStateException("Unsupported family " + _family); } + default -> throw new IllegalStateException("Unsupported family " + _family); } proxyEndPoint = new ProxyEndPoint(endPoint, local, remote); @@ -714,37 +710,20 @@ public class ProxyConnectionFactory extends DetectorConnectionFactory int transportAndFamily = 0xFF & byteBuffer.get(); switch (transportAndFamily >> 4) { - case 0: - _family = Family.UNSPEC; - break; - case 1: - _family = Family.INET; - break; - case 2: - _family = Family.INET6; - break; - case 3: - _family = Family.UNIX; - break; - default: - throw new IOException("Proxy v2 bad PROXY family"); + case 0 -> _family = Family.UNSPEC; + case 1 -> _family = Family.INET; + case 2 -> _family = Family.INET6; + case 3 -> _family = Family.UNIX; + default -> throw new IOException("Proxy v2 bad PROXY family"); } - Transport transport; - switch (transportAndFamily & 0xF) + Transport transport = switch (transportAndFamily & 0xF) { - case 0: - transport = Transport.UNSPEC; - break; - case 1: - transport = Transport.STREAM; - break; - case 2: - transport = Transport.DGRAM; - break; - default: - throw new IOException("Proxy v2 bad PROXY family"); - } + case 0 -> Transport.UNSPEC; + case 1 -> Transport.STREAM; + case 2 -> Transport.DGRAM; + default -> throw new IOException("Proxy v2 bad PROXY family"); + }; _length = byteBuffer.getChar(); @@ -761,6 +740,8 @@ public class ProxyConnectionFactory extends DetectorConnectionFactory private void releaseAndClose() { + if (LOG.isDebugEnabled()) + LOG.debug("Proxy v2 releasing buffer and closing"); _buffer.release(); close(); } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartByteRangesTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartByteRangesTest.java index 745de4c3ada..8c138113014 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartByteRangesTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartByteRangesTest.java @@ -19,7 +19,6 @@ import java.nio.channels.SocketChannel; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.jetty.http.ByteRange; import org.eclipse.jetty.http.HttpField; @@ -31,7 +30,6 @@ import org.eclipse.jetty.http.MultiPart; import org.eclipse.jetty.http.MultiPartByteRanges; import org.eclipse.jetty.io.ArrayByteBufferPool; import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.io.content.ByteBufferContentSource; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; @@ -49,13 +47,13 @@ public class MultiPartByteRangesTest { private Server server; private ServerConnector connector; - private LeakTrackingBufferPool byteBufferPool; + private ArrayByteBufferPool.Tracking byteBufferPool; private void start(Handler handler) throws Exception { QueuedThreadPool serverThreads = new QueuedThreadPool(); serverThreads.setName("server"); - byteBufferPool = new LeakTrackingBufferPool(); + byteBufferPool = new ArrayByteBufferPool.Tracking(); server = new Server(serverThreads, null, byteBufferPool); connector = new ServerConnector(server, 1, 1); server.addConnector(connector); @@ -67,7 +65,7 @@ public class MultiPartByteRangesTest public void dispose() { LifeCycle.stop(server); - assertEquals(0, byteBufferPool.countLeaks()); + assertEquals(0, byteBufferPool.getLeaks().size()); } @Test @@ -131,31 +129,4 @@ public class MultiPartByteRangesTest assertEquals("CDEF", Content.Source.asString(part3.getContentSource())); } } - - private static class LeakTrackingBufferPool extends ArrayByteBufferPool - { - private final AtomicInteger leaks = new AtomicInteger(); - - public int countLeaks() - { - return leaks.get(); - } - - @Override - public RetainableByteBuffer acquire(int size, boolean direct) - { - leaks.incrementAndGet(); - return new RetainableByteBuffer.Wrapper(super.acquire(size, direct)) - { - @Override - public boolean release() - { - boolean released = super.release(); - if (released) - leaks.decrementAndGet(); - return released; - } - }; - } - } } From 24792db09c86ff1c7394227e9fbd4feecb6b9706 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 8 Aug 2023 16:16:40 +0200 Subject: [PATCH 20/31] Fixes #10218 - NPE in HttpChannelOverFCGI.receive() Simplified the code, removing all leftover cruft present since FCGI was multiplexed. Now the FCGI implementation is very similar to HTTP1. Made HttpConnectionOverFCGI.channel final so that it cannot NPE anymore. The client now properly handling server-side connection closes. Signed-off-by: Simone Bordet --- .../internal/HttpChannelOverFCGI.java | 82 ++----- .../internal/HttpConnectionOverFCGI.java | 213 +++++------------- .../internal/HttpReceiverOverFCGI.java | 11 + .../org/eclipse/jetty/fcgi/parser/Parser.java | 22 +- .../server/internal/ServerFCGIConnection.java | 5 + .../jetty/fcgi/server/HttpClientTest.java | 37 ++- .../test/client/transport/AbstractTest.java | 5 + .../transport/HttpChannelAssociationTest.java | 5 +- 8 files changed, 145 insertions(+), 235 deletions(-) diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpChannelOverFCGI.java b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpChannelOverFCGI.java index 4a80780ca81..27678000d57 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpChannelOverFCGI.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpChannelOverFCGI.java @@ -13,45 +13,33 @@ package org.eclipse.jetty.fcgi.client.transport.internal; -import java.util.concurrent.TimeoutException; - import org.eclipse.jetty.client.Result; import org.eclipse.jetty.client.transport.HttpChannel; import org.eclipse.jetty.client.transport.HttpExchange; import org.eclipse.jetty.client.transport.HttpReceiver; import org.eclipse.jetty.client.transport.HttpSender; -import org.eclipse.jetty.fcgi.generator.Flusher; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.io.IdleTimeout; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Promise; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class HttpChannelOverFCGI extends HttpChannel { - private static final Logger LOG = LoggerFactory.getLogger(HttpChannelOverFCGI.class); - private final HttpConnectionOverFCGI connection; - private final Flusher flusher; private final HttpSenderOverFCGI sender; private final HttpReceiverOverFCGI receiver; - private final FCGIIdleTimeout idle; private int request; private HttpVersion version; - public HttpChannelOverFCGI(HttpConnectionOverFCGI connection, Flusher flusher, long idleTimeout) + public HttpChannelOverFCGI(HttpConnectionOverFCGI connection) { super(connection.getHttpDestination()); this.connection = connection; - this.flusher = flusher; this.sender = new HttpSenderOverFCGI(this); this.receiver = new HttpReceiverOverFCGI(this); - this.idle = new FCGIIdleTimeout(connection, idleTimeout); } public HttpConnectionOverFCGI getHttpConnection() @@ -81,28 +69,21 @@ public class HttpChannelOverFCGI extends HttpChannel return receiver; } - public boolean isFailed() - { - return sender.isFailed() || receiver.isFailed(); - } - @Override public void send(HttpExchange exchange) { version = exchange.getRequest().getVersion(); - idle.onOpen(); sender.send(exchange); } @Override public void release() { - connection.release(this); + connection.release(); } protected void responseBegin(int code, String reason) { - idle.notIdle(); HttpExchange exchange = getHttpExchange(); if (exchange == null) return; @@ -119,7 +100,6 @@ public class HttpChannelOverFCGI extends HttpChannel protected void responseHeaders() { - idle.notIdle(); HttpExchange exchange = getHttpExchange(); if (exchange != null) receiver.responseHeaders(exchange); @@ -127,7 +107,6 @@ public class HttpChannelOverFCGI extends HttpChannel protected void content(Content.Chunk chunk) { - idle.notIdle(); HttpExchange exchange = getHttpExchange(); if (exchange != null) receiver.content(chunk); @@ -135,7 +114,6 @@ public class HttpChannelOverFCGI extends HttpChannel protected void end() { - idle.notIdle(); HttpExchange exchange = getHttpExchange(); if (exchange != null) receiver.end(exchange); @@ -150,67 +128,33 @@ public class HttpChannelOverFCGI extends HttpChannel promise.succeeded(false); } + void eof() + { + HttpExchange exchange = getHttpExchange(); + if (exchange == null) + connection.close(); + } + @Override public void exchangeTerminated(HttpExchange exchange, Result result) { super.exchangeTerminated(exchange, result); - idle.onClose(); HttpFields responseHeaders = result.getResponse().getHeaders(); if (result.isFailed()) connection.close(result.getFailure()); - else if (!connection.closeByHTTP(responseHeaders)) + else if (connection.isShutdown() || connection.isCloseByHTTP(responseHeaders)) + connection.close(); + else release(); } protected void flush(ByteBufferPool.Accumulator accumulator, Callback callback) { - flusher.flush(accumulator, callback); + connection.getFlusher().flush(accumulator, callback); } void receive() { receiver.receive(); } - - private class FCGIIdleTimeout extends IdleTimeout - { - private final HttpConnectionOverFCGI connection; - private boolean open; - - public FCGIIdleTimeout(HttpConnectionOverFCGI connection, long idleTimeout) - { - super(connection.getHttpDestination().getHttpClient().getScheduler()); - this.connection = connection; - setIdleTimeout(idleTimeout >= 0 ? idleTimeout : connection.getEndPoint().getIdleTimeout()); - } - - @Override - public void onOpen() - { - open = true; - notIdle(); - super.onOpen(); - } - - @Override - public void onClose() - { - super.onClose(); - open = false; - } - - @Override - protected void onIdleExpired(TimeoutException timeout) - { - if (LOG.isDebugEnabled()) - LOG.debug("Idle timeout for request {}", request); - connection.abort(timeout); - } - - @Override - public boolean isOpen() - { - return open; - } - } } diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpConnectionOverFCGI.java b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpConnectionOverFCGI.java index 4a28e87880c..7ec4dc64ddd 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpConnectionOverFCGI.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpConnectionOverFCGI.java @@ -13,14 +13,13 @@ package org.eclipse.jetty.fcgi.client.transport.internal; -import java.io.EOFException; import java.nio.ByteBuffer; import java.nio.channels.AsynchronousCloseException; import java.util.Collections; import java.util.Iterator; -import java.util.LinkedList; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.jetty.client.Connection; import org.eclipse.jetty.client.Destination; @@ -49,7 +48,6 @@ import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.Attachable; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Promise; -import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,18 +56,19 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne private static final Logger LOG = LoggerFactory.getLogger(HttpConnectionOverFCGI.class); private final ByteBufferPool networkByteBufferPool; - private final AutoLock lock = new AutoLock(); - private final LinkedList requests = new LinkedList<>(); + private final AtomicInteger requests = new AtomicInteger(); private final AtomicBoolean closed = new AtomicBoolean(); private final HttpDestination destination; private final Promise promise; private final Flusher flusher; private final Delegate delegate; private final ClientParser parser; - private HttpChannelOverFCGI channel; + private final HttpChannelOverFCGI channel; private RetainableByteBuffer networkBuffer; private Object attachment; private Runnable action; + private long idleTimeout; + private boolean shutdown; public HttpConnectionOverFCGI(EndPoint endPoint, Destination destination, Promise promise) { @@ -79,7 +78,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne this.flusher = new Flusher(endPoint); this.delegate = new Delegate(destination); this.parser = new ClientParser(new ResponseListener()); - requests.addLast(0); + this.channel = newHttpChannel(); HttpClient client = destination.getHttpClient(); this.networkByteBufferPool = client.getByteBufferPool(); } @@ -207,13 +206,18 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne private void shutdown() { - // Close explicitly only if we are idle, since the request may still - // be in progress, otherwise close only if we can fail the responses. - HttpChannelOverFCGI channel = this.channel; - if (channel == null || channel.getRequest() == 0) - close(); - else - failAndClose(new EOFException(String.valueOf(getEndPoint()))); + // Mark this receiver as shutdown, so that we can + // close the connection when the exchange terminates. + // We cannot close the connection from here because + // the request may still be in process. + shutdown = true; + if (!parser.eof()) + channel.eof(); + } + + boolean isShutdown() + { + return shutdown; } @Override @@ -226,27 +230,11 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne return false; } - protected void release(HttpChannelOverFCGI channel) + protected void release() { - HttpChannelOverFCGI existing = this.channel; - if (existing == channel) - { - channel.setRequest(0); - // Recycle only non-failed channels. - if (channel.isFailed()) - { - channel.destroy(); - this.channel = null; - } - destination.release(this); - } - else - { - if (existing == null) - channel.destroy(); - else - throw new UnsupportedOperationException("FastCGI Multiplex"); - } + // Restore idle timeout + getEndPoint().setIdleTimeout(idleTimeout); + destination.release(this); } @Override @@ -260,9 +248,8 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne if (closed.compareAndSet(false, true)) { getHttpDestination().remove(this); - abort(failure); - + channel.destroy(); getEndPoint().shutdownOutput(); if (LOG.isDebugEnabled()) LOG.debug("Shutdown {}", this); @@ -290,62 +277,25 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne return attachment; } - protected boolean closeByHTTP(HttpFields fields) + protected boolean isCloseByHTTP(HttpFields fields) { - if (!fields.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString())) - return false; - close(); - return true; + return fields.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString()); } protected void abort(Throwable failure) { - HttpChannelOverFCGI channel = this.channel; - if (channel != null) - { - HttpExchange exchange = channel.getHttpExchange(); - if (exchange != null) - exchange.getRequest().abort(failure); - channel.destroy(); - this.channel = null; - } + HttpExchange exchange = channel.getHttpExchange(); + if (exchange != null) + exchange.getRequest().abort(failure); } private void failAndClose(Throwable failure) { - HttpChannelOverFCGI channel = this.channel; - if (channel != null) + channel.responseFailure(failure, Promise.from(failed -> { - channel.responseFailure(failure, Promise.from(failed -> - { - channel.destroy(); - if (failed) - close(failure); - }, x -> - { - channel.destroy(); + if (failed) close(failure); - })); - } - } - - private int acquireRequest() - { - try (AutoLock ignored = lock.lock()) - { - int last = requests.getLast(); - int request = last + 1; - requests.addLast(request); - return request; - } - } - - private void releaseRequest(int request) - { - try (AutoLock ignored = lock.lock()) - { - requests.removeFirstOccurrence(request); - } + }, x -> close(failure))); } private Runnable getAndSetAction(Runnable action) @@ -355,17 +305,9 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne return r; } - protected HttpChannelOverFCGI acquireHttpChannel(int id, Request request) + protected HttpChannelOverFCGI newHttpChannel() { - if (channel == null) - channel = newHttpChannel(request); - channel.setRequest(id); - return channel; - } - - protected HttpChannelOverFCGI newHttpChannel(Request request) - { - return new HttpChannelOverFCGI(this, getFlusher(), request.getIdleTimeout()); + return new HttpChannelOverFCGI(this); } @Override @@ -388,8 +330,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne @Override protected Iterator getHttpChannels() { - HttpChannel channel = HttpConnectionOverFCGI.this.channel; - return channel == null ? Collections.emptyIterator() : Collections.singleton(channel).iterator(); + return Collections.singleton(channel).iterator(); } @Override @@ -398,9 +339,14 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne HttpRequest request = exchange.getRequest(); normalizeRequest(request); - int id = acquireRequest(); - HttpChannelOverFCGI channel = acquireHttpChannel(id, request); + // Save the old idle timeout to restore it. + EndPoint endPoint = getEndPoint(); + idleTimeout = endPoint.getIdleTimeout(); + long requestIdleTimeout = request.getIdleTimeout(); + if (requestIdleTimeout >= 0) + endPoint.setIdleTimeout(requestIdleTimeout); + channel.setRequest(requests.incrementAndGet()); return send(channel, exchange); } @@ -431,11 +377,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne { if (LOG.isDebugEnabled()) LOG.debug("onBegin r={},c={},reason={}", request, code, reason); - HttpChannelOverFCGI channel = HttpConnectionOverFCGI.this.channel; - if (channel != null) - channel.responseBegin(code, reason); - else - noChannel(request); + channel.responseBegin(code, reason); } @Override @@ -443,11 +385,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne { if (LOG.isDebugEnabled()) LOG.debug("onHeader r={},f={}", request, field); - HttpChannelOverFCGI channel = HttpConnectionOverFCGI.this.channel; - if (channel != null) - channel.responseHeader(field); - else - noChannel(request); + channel.responseHeader(field); } @Override @@ -455,15 +393,9 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne { if (LOG.isDebugEnabled()) LOG.debug("onHeaders r={} {}", request, networkBuffer); - HttpChannelOverFCGI channel = HttpConnectionOverFCGI.this.channel; - if (channel != null) - { - if (getAndSetAction(channel::responseHeaders) != null) - throw new IllegalStateException(); - return true; - } - noChannel(request); - return false; + if (getAndSetAction(channel::responseHeaders) != null) + throw new IllegalStateException(); + return true; } @Override @@ -475,21 +407,13 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne { case STD_OUT -> { - HttpChannelOverFCGI channel = HttpConnectionOverFCGI.this.channel; - if (channel != null) - { - // No need to call networkBuffer.retain() here, since we know - // that the action will be run before releasing the networkBuffer. - // The receiver of the chunk decides whether to consume/retain it. - Content.Chunk chunk = Content.Chunk.asChunk(buffer, false, networkBuffer); - if (getAndSetAction(() -> channel.content(chunk)) == null) - return true; - throw new IllegalStateException(); - } - else - { - noChannel(request); - } + // No need to call networkBuffer.retain() here, since we know + // that the action will be run before releasing the networkBuffer. + // The receiver of the chunk decides whether to consume/retain it. + Content.Chunk chunk = Content.Chunk.asChunk(buffer, false, networkBuffer); + if (getAndSetAction(() -> channel.content(chunk)) == null) + return true; + throw new IllegalStateException(); } case STD_ERR -> LOG.info(BufferUtil.toUTF8String(buffer)); default -> throw new IllegalArgumentException(); @@ -502,16 +426,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne { if (LOG.isDebugEnabled()) LOG.debug("onEnd r={}", request); - HttpChannelOverFCGI channel = HttpConnectionOverFCGI.this.channel; - if (channel != null) - { - releaseRequest(request); - channel.end(); - } - else - { - noChannel(request); - } + channel.end(); } @Override @@ -519,25 +434,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne { if (LOG.isDebugEnabled()) LOG.debug("onFailure request={}", request, failure); - HttpChannelOverFCGI channel = HttpConnectionOverFCGI.this.channel; - if (channel != null) - { - channel.responseFailure(failure, Promise.from(failed -> - { - if (failed) - releaseRequest(request); - }, x -> releaseRequest(request))); - } - else - { - noChannel(request); - } - } - - private void noChannel(int request) - { - if (LOG.isDebugEnabled()) - LOG.debug("Channel not found for request {}", request); + failAndClose(failure); } } } diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpReceiverOverFCGI.java b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpReceiverOverFCGI.java index a59016f108f..b904a8347c1 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpReceiverOverFCGI.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpReceiverOverFCGI.java @@ -61,6 +61,17 @@ public class HttpReceiverOverFCGI extends HttpReceiver } } + @Override + protected void dispose() + { + super.dispose(); + if (chunk != null) + { + chunk.release(); + chunk = null; + } + } + @Override public Content.Chunk read(boolean fillInterestIfNeeded) { diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/Parser.java b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/Parser.java index c410c9a9117..a45b17a7deb 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/Parser.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/Parser.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.fcgi.parser; +import java.io.EOFException; import java.nio.ByteBuffer; import org.eclipse.jetty.fcgi.FCGI; @@ -60,7 +61,7 @@ public abstract class Parser protected final HeaderParser headerParser = new HeaderParser(); private final Listener listener; - private State state = State.HEADER; + private State state = State.INITIAL; private int padding; protected Parser(Listener listener) @@ -80,6 +81,12 @@ public abstract class Parser { switch (state) { + case INITIAL -> + { + if (!buffer.hasRemaining()) + return false; + state = State.HEADER; + } case HEADER -> { if (!headerParser.parse(buffer)) @@ -145,10 +152,19 @@ public abstract class Parser protected abstract ContentParser findContentParser(FCGI.FrameType frameType); + public boolean eof() + { + if (state == State.INITIAL) + return false; + Throwable failure = new EOFException(); + listener.onFailure(headerParser.getRequest(), failure); + return true; + } + private void reset() { headerParser.reset(); - state = State.HEADER; + state = State.INITIAL; padding = 0; } @@ -190,6 +206,6 @@ public abstract class Parser private enum State { - HEADER, CONTENT, PADDING + INITIAL, HEADER, CONTENT, PADDING } } diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java index ef2cac46f60..8f517c09014 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java @@ -270,6 +270,8 @@ public class ServerFCGIConnection extends AbstractConnection implements Connecti private void releaseInputBuffer() { + if (networkBuffer == null) + return; boolean released = networkBuffer.release(); if (LOG.isDebugEnabled()) LOG.debug("releaseInputBuffer {} {}", released, this); @@ -327,6 +329,9 @@ public class ServerFCGIConnection extends AbstractConnection implements Connecti @Override public boolean onIdleExpired(TimeoutException timeoutException) { + HttpStreamOverFCGI stream = this.stream; + if (stream == null) + return true; Runnable task = stream.getHttpChannel().onIdleTimeout(timeoutException); if (task != null) getExecutor().execute(task); diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java index 83c3a75cb55..fba92ebd364 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java @@ -32,7 +32,9 @@ import java.util.zip.GZIPOutputStream; import org.eclipse.jetty.client.AsyncRequestContent; import org.eclipse.jetty.client.BytesRequestContent; +import org.eclipse.jetty.client.ConnectionPool; import org.eclipse.jetty.client.ContentResponse; +import org.eclipse.jetty.client.Destination; import org.eclipse.jetty.client.FutureResponseListener; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.Request; @@ -515,6 +517,35 @@ public class HttpClientTest extends AbstractHttpClientServerTest @Test public void testConnectionIdleTimeout() throws Exception + { + long idleTimeout = 1000; + start(new Handler.Abstract() + { + @Override + public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jetty.server.Response response, Callback callback) + { + callback.succeeded(); + return true; + } + }); + connector.setIdleTimeout(idleTimeout); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scheme) + .timeout(2 * idleTimeout, TimeUnit.MILLISECONDS) + .send(); + assertNotNull(response); + assertEquals(200, response.getStatus()); + + Thread.sleep(2 * idleTimeout); + + assertTrue(client.getDestinations().stream() + .map(Destination::getConnectionPool) + .allMatch(ConnectionPool::isEmpty)); + } + + @Test + public void testConnectionIdleTimeoutIgnored() throws Exception { long idleTimeout = 1000; start(new Handler.Abstract() @@ -522,9 +553,11 @@ public class HttpClientTest extends AbstractHttpClientServerTest @Override public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception { - // Handler says it will handle the idletimeout + // Handler says it will handle the idle timeout by ignoring it. request.addIdleTimeoutListener(t -> false); - TimeUnit.MILLISECONDS.sleep(2 * idleTimeout); + // Sleep an non-integral number of idle timeouts to avoid + // racing with the idle timeout ticking every idle period. + TimeUnit.MILLISECONDS.sleep(idleTimeout * 3 / 2); callback.succeeded(); return true; } diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/AbstractTest.java b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/AbstractTest.java index 4e3fc47e195..148b3b3d6af 100644 --- a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/AbstractTest.java +++ b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/AbstractTest.java @@ -59,11 +59,16 @@ import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.extension.BeforeTestExecutionCallback; +import org.junit.jupiter.api.extension.RegisterExtension; import static org.junit.jupiter.api.Assertions.assertTrue; public class AbstractTest { + @RegisterExtension + public final BeforeTestExecutionCallback printMethodName = context -> + System.err.printf("Running %s.%s() %s%n", context.getRequiredTestClass().getSimpleName(), context.getRequiredTestMethod().getName(), context.getDisplayName()); protected final HttpConfiguration httpConfig = new HttpConfiguration(); protected SslContextFactory.Server sslContextFactoryServer; protected Server server; diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpChannelAssociationTest.java b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpChannelAssociationTest.java index a3b73d19dde..676f68bb0a7 100644 --- a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpChannelAssociationTest.java +++ b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpChannelAssociationTest.java @@ -22,7 +22,6 @@ import org.eclipse.jetty.client.Connection; import org.eclipse.jetty.client.Destination; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.HttpClientTransport; -import org.eclipse.jetty.client.Request; import org.eclipse.jetty.client.transport.HttpClientTransportOverHTTP; import org.eclipse.jetty.client.transport.HttpExchange; import org.eclipse.jetty.client.transport.internal.HttpChannelOverHTTP; @@ -209,9 +208,9 @@ public class HttpChannelAssociationTest extends AbstractTest return new HttpConnectionOverFCGI(endPoint, destination, promise) { @Override - protected HttpChannelOverFCGI newHttpChannel(Request request) + protected HttpChannelOverFCGI newHttpChannel() { - return new HttpChannelOverFCGI(this, getFlusher(), request.getIdleTimeout()) + return new HttpChannelOverFCGI(this) { @Override public boolean associate(HttpExchange exchange) From fd1413db58dd392ae11507da6c83fd097fdb146a Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 16 Aug 2023 16:09:36 -0500 Subject: [PATCH 21/31] Issue #10274 - Improve javadoc + move internal methods (#10276) * Improve javadoc + move internal methods --- .../java/org/eclipse/jetty/util/URIUtil.java | 3 +- .../jetty/util/resource/ResourceFactory.java | 220 +++++++++++++----- .../resource/ResourceFactoryInternals.java | 36 ++- .../jetty/util/resource/package-info.java | 12 +- 4 files changed, 205 insertions(+), 66 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java index 36ff51491f2..c49049c38d9 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java @@ -1836,8 +1836,7 @@ public final class URIUtil Objects.requireNonNull(resource); // Only try URI for string for known schemes, otherwise assume it is a Path - ResourceFactory resourceFactory = ResourceFactory.getBestByScheme(resource); - return (resourceFactory != null) + return (ResourceFactory.isSupported(resource)) ? correctFileURI(URI.create(resource)) : Paths.get(resource).toUri(); } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java index 300d2335175..6927c09295e 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java @@ -27,7 +27,89 @@ import org.eclipse.jetty.util.component.Container; import org.eclipse.jetty.util.component.Dumpable; /** - * ResourceFactory. + *

ResourceFactory is the source of new {@link Resource} instances.

+ * + *

+ * Some {@link Resource} objects have an internal allocation / release model, + * that the {@link ResourceFactory} is responsible for. + * Once a {@link ResourceFactory} is stopped, the {@link Resource} + * objects created from that {@link ResourceFactory} are released. + *

+ * + *

A {@link ResourceFactory.LifeCycle} tied to a Jetty {@link org.eclipse.jetty.util.component.Container}

+ * 
+ *     ResourceFactory.LifeCycle resourceFactory = ResourceFactory.of(container);
+ *     Resource resource = resourceFactory.newResource(ref);
+ *
+ *

+ * The use of {@link ResourceFactory#of(Container)} results in a {@link ResourceFactory.LifeCycle} that is tied + * to a specific Jetty {@link org.eclipse.jetty.util.component.Container} such as a {@code Server}, + * {@code ServletContextHandler}, or {@code WebAppContext}. This will free the {@code Resource} + * instances created by the {@link org.eclipse.jetty.util.resource.ResourceFactory} once + * the {@code container} that manages it is stopped. + *

+ * + *

A {@link ResourceFactory.Closeable} that exists within a {@code try-with-resources} call

+ * 
+ *     try (ResourceFactory.Closeable resourceFactory = ResourceFactory.closeable()) {
+ *         Resource resource = resourceFactory.newResource(ref);
+ *     }
+ *
+ *

+ * The use of {@link ResourceFactory#closeable()} results in a {@link ResourceFactory} that only exists for + * the duration of the {@code try-with-resources} code block, once this {@code try-with-resources} is closed, + * all {@link Resource} objects associated with that {@link ResourceFactory} are freed as well. + *

+ * + *

A {@code ResourceFactory} that lives at the JVM level

+ * 
+ *     ResourceFactory resourceFactory = ResourceFactory.root();
+ *     Resource resource = resourceFactory.newResource(ref);
+ *
+ *

+ * The use of {@link ResourceFactory#root()} results in a {@link ResourceFactory} that exists for + * the life of the JVM, and the resources allocated via this {@link ResourceFactory} will not + * be freed until the JVM exits. + *

+ * + *

Supported URI Schemes

+ *

+ * By default, the following schemes are supported by Jetty. + *

+ *
+ *
file
+ *
The standard Java {@code file:/path/to/dir/} syntax
+ * + *
jar
+ *
The standard Java {@code jar:file:/path/to/file.jar!/} syntax
+ * + *
jrt
+ *
The standard Java {@code jrt:module-name} syntax
+ *
+ *

+ * Special Note: An effort is made to discover any new schemes that + * might be present at JVM startup (eg: graalvm and {@code resource:} scheme). + * At startup Jetty will access an internal Jetty resource (found in + * the jetty-util jar) and seeing what {@code scheme} it is using to access + * it, and will register it with a call to + * {@link ResourceFactory#registerResourceFactory(String, ResourceFactory)}. + *

+ * + *

Supporting more Schemes

+ *

+ * You can register a new URI scheme to a {@link ResourceFactory} implementation + * using the {@link ResourceFactory#registerResourceFactory(String, ResourceFactory)} + * method, which will cause all new uses of ResourceFactory to use this newly + * registered scheme. + *

+ * 
+ *     URLResourceFactory urlResourceFactory = new URLResourceFactory();
+ *     urlResourceFactory.setConnectTimeout(1000);
+ *     ResourceFactory.registerResourceFactory("https", urlResourceFactory);
+ *
+ *     URI web = URI.create("https://eclipse.dev/jetty/");
+ *     Resource resource = ResourceFactory.root().newResource(web);
+ *
*/ public interface ResourceFactory { @@ -39,7 +121,6 @@ public interface ResourceFactory * or null if none are passed. * The returned {@link Resource} will always return {@code true} from {@link Resource#isDirectory()} * @throws IllegalArgumentException if a non-directory resource is passed. - * @see CombinedResource */ static Resource combine(List resources) { @@ -54,7 +135,6 @@ public interface ResourceFactory * or null if none are passed. * The returned {@link Resource} will always return {@code true} from {@link Resource#isDirectory()} * @throws IllegalArgumentException if a non-directory resource is passed. - * @see CombinedResource */ static Resource combine(Resource... resources) { @@ -65,17 +145,21 @@ public interface ResourceFactory * Construct a resource from a uri. * * @param uri A URI. - * @return A Resource object. + * @return A Resource object, or null if uri points to a location that does not exist. */ Resource newResource(URI uri); /** - * Construct a system resource from a string. - * The resource is tried as classloader resource before being - * treated as a normal resource. + *

Construct a system resource from a string.

+ * + *

+ * The resource is first attempted to be accessed via the {@link Thread#getContextClassLoader()} + * before being treated as a normal resource. + *

* * @param resource Resource as string representation - * @return The new Resource + * @return The new Resource, or null if string points to a location that does not exist + * @throws IllegalArgumentException if string is blank */ default Resource newSystemResource(String resource) { @@ -134,12 +218,16 @@ public interface ResourceFactory } /** - * Find a classpath resource. + *

Find a classpath resource.

+ * + *

* The {@link Class#getResource(String)} method is used to lookup the resource. If it is not * found, then the {@link Loader#getResource(String)} method is used. + *

* * @param resource the relative name of the resource - * @return Resource or null + * @return Resource, or null if string points to a location that does not exist + * @throws IllegalArgumentException if string is blank */ default Resource newClassPathResource(String resource) { @@ -164,9 +252,20 @@ public interface ResourceFactory } /** + *

* Load a URL into a memory resource. + *

+ * + *

+ * A Memory Resource is created from a the contents of + * {@link URL#openStream()} and kept in memory from + * that point forward. Never accessing the URL + * again to refresh it's contents. + *

+ * * @param url the URL to load into memory - * @return Resource or null + * @return Resource, or null if url points to a location that does not exist + * @throws IllegalArgumentException if URL is null * @see #newClassPathResource(String) */ default Resource newMemoryResource(URL url) @@ -181,7 +280,9 @@ public interface ResourceFactory * Construct a resource from a string. * * @param resource A URL or filename. - * @return A Resource object. + * @return A Resource object, or null if the string points to a location + * that does not exist + * @throws IllegalArgumentException if resource is invalid */ default Resource newResource(String resource) { @@ -192,10 +293,12 @@ public interface ResourceFactory } /** - * Construct a Resource from provided path + * Construct a Resource from provided path. * * @param path the path - * @return the Resource for the provided path + * @return the Resource for the provided path, or null if the path + * does not exist + * @throws IllegalArgumentException if path is null */ default Resource newResource(Path path) { @@ -206,10 +309,12 @@ public interface ResourceFactory } /** - * Construct a possible {@link CombinedResource} from a list of URIs + * Construct a possible combined {@code Resource} from a list of URIs. * * @param uris the URIs - * @return the Resource for the provided path + * @return the Resource for the provided URIs, or null if all + * of the provided URIs do not exist + * @throws IllegalArgumentException if list of URIs is empty or null */ default Resource newResource(List uris) { @@ -220,10 +325,12 @@ public interface ResourceFactory } /** - * Construct a {@link Resource} from a provided URL + * Construct a {@link Resource} from a provided URL. * * @param url the URL - * @return the Resource for the provided URL + * @return the Resource for the provided URL, or null if the + * url points to a location that does not exist + * @throws IllegalArgumentException if url is null */ default Resource newResource(URL url) { @@ -241,10 +348,12 @@ public interface ResourceFactory } /** - * Construct a {@link Resource} from a {@code file:} based URI that is mountable (eg: a jar file) + * Construct a {@link Resource} from a {@code file:} based URI that is mountable (eg: a jar file). * * @param uri the URI - * @return the Resource, mounted as a {@link java.nio.file.FileSystem} + * @return the Resource, mounted as a {@link java.nio.file.FileSystem}, or null if + * the uri points to a location that does not exist. + * @throws IllegalArgumentException if provided URI is not "file" scheme. */ default Resource newJarFileResource(URI uri) { @@ -253,6 +362,28 @@ public interface ResourceFactory return newResource(URIUtil.toJarFileUri(uri)); } + /** + * Test to see if provided string is supported. + * + * @param str the string to test + * @return true if it is supported + */ + static boolean isSupported(String str) + { + return ResourceFactoryInternals.isSupported(str); + } + + /** + * Test to see if provided uri is supported. + * + * @param uri the uri to test + * @return true if it is supported + */ + static boolean isSupported(URI uri) + { + return ResourceFactoryInternals.isSupported(uri); + } + /** * Register a new ResourceFactory that can handle the specific scheme for the Resource API. * @@ -262,12 +393,14 @@ public interface ResourceFactory * * @param scheme the scheme to support (eg: `ftp`, `http`, `resource`, etc) * @param resourceFactory the ResourceFactory to be responsible for the registered scheme. + * @throws IllegalArgumentException if scheme is blank * @see #unregisterResourceFactory(String) - * @see #byScheme(String) - * @see #getBestByScheme(String) */ static void registerResourceFactory(String scheme, ResourceFactory resourceFactory) { + if (StringUtil.isBlank(scheme)) + throw new IllegalArgumentException("Scheme is blank"); + ResourceFactoryInternals.RESOURCE_FACTORIES.put(scheme, resourceFactory); } @@ -276,53 +409,16 @@ public interface ResourceFactory * * @param scheme the scheme to unregister * @return the existing {@link ResourceFactory} that was registered to that scheme. + * @throws IllegalArgumentException if scheme is blank * @see #registerResourceFactory(String, ResourceFactory) - * @see #byScheme(String) - * @see #getBestByScheme(String) */ static ResourceFactory unregisterResourceFactory(String scheme) { + if (StringUtil.isBlank(scheme)) + throw new IllegalArgumentException("Scheme is blank"); return ResourceFactoryInternals.RESOURCE_FACTORIES.remove(scheme); } - /** - * Get the {@link ResourceFactory} that is registered for the specific scheme. - * - * 
{@code
-     * .byScheme("jar") == ResourceFactory supporting jar
-     * .byScheme("jar:file://foo.jar!/") == null // full url strings not supported)
-     * }
- * - * @param scheme the scheme to look up - * @return the {@link ResourceFactory} responsible for the scheme, null if no {@link ResourceFactory} handles the scheme. - * @see #registerResourceFactory(String, ResourceFactory) - * @see #unregisterResourceFactory(String) - * @see #getBestByScheme(String) - */ - static ResourceFactory byScheme(String scheme) - { - return ResourceFactoryInternals.RESOURCE_FACTORIES.get(scheme); - } - - /** - * Get the best ResourceFactory for the provided scheme. - * - *

- * Unlike {@link #byScheme(String)}, this supports arbitrary Strings, that might start with a supported scheme. - *

- * - * @param scheme the scheme to look up - * @return the ResourceFactory that best fits the provided scheme. - * @see org.eclipse.jetty.util.Index#getBest(String) - * @see #registerResourceFactory(String, ResourceFactory) - * @see #unregisterResourceFactory(String) - * @see #byScheme(String) - */ - static ResourceFactory getBestByScheme(String scheme) - { - return ResourceFactoryInternals.RESOURCE_FACTORIES.getBest(scheme); - } - /** * The JVM wide (root) ResourceFactory. * diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java index 595ca615feb..1bc7728382f 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java @@ -24,6 +24,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.Index; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.component.Dumpable; @@ -93,6 +94,39 @@ class ResourceFactoryInternals } }; + /** + * Test uri to know if a {@link ResourceFactory} is registered for it. + * + * @param uri the uri to test + * @return true if a ResourceFactory is registered to support the uri + * @see ResourceFactory#registerResourceFactory(String, ResourceFactory) + * @see ResourceFactory#unregisterResourceFactory(String) + * @see #isSupported(String) + */ + static boolean isSupported(URI uri) // TODO: boolean isSupported + { + if (uri == null || uri.getScheme() == null) + return false; + return RESOURCE_FACTORIES.get(uri.getScheme()) != null; + } + + /** + * Test string to know if a {@link ResourceFactory} is registered for it. + * + * @param str the string representing the resource location + * @return true if a ResourceFactory is registered to support the string representation + * @see org.eclipse.jetty.util.Index#getBest(String) + * @see ResourceFactory#registerResourceFactory(String, ResourceFactory) + * @see ResourceFactory#unregisterResourceFactory(String) + * @see #isSupported(URI) + */ + static boolean isSupported(String str) + { + if (StringUtil.isBlank(str)) + return false; + return RESOURCE_FACTORIES.getBest(str) != null; + } + static class Closeable implements ResourceFactory.Closeable { private final CompositeResourceFactory _compositeResourceFactory = new CompositeResourceFactory(); @@ -168,7 +202,7 @@ class ResourceFactoryInternals uri = URIUtil.correctFileURI(uri); } - ResourceFactory resourceFactory = ResourceFactory.byScheme(uri.getScheme()); + ResourceFactory resourceFactory = RESOURCE_FACTORIES.get(uri.getScheme()); if (resourceFactory == null) throw new IllegalArgumentException("URI scheme not supported: " + uri); if (resourceFactory instanceof MountedPathResourceFactory) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/package-info.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/package-info.java index 9102cdf6abb..c1fe89fd4c3 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/package-info.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/package-info.java @@ -12,7 +12,17 @@ // /** - * Jetty Util : Common Resource Utilities + *

Jetty Util : Resource Utilities

+ * + *

+ * A {@link org.eclipse.jetty.util.resource.Resource} in Jetty is an abstraction that + * allows for a common API to access various forms of resource sources across the Jetty + * ecosystem. + *

+ *

+ * A {@link org.eclipse.jetty.util.resource.Resource} is created via one of the + * {@link org.eclipse.jetty.util.resource.ResourceFactory}{@code .newResource(...)} APIs. + *

*/ package org.eclipse.jetty.util.resource; From 6702634494d0ca705604547f059d1fe3b35931c3 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 17 Aug 2023 07:37:40 -0500 Subject: [PATCH 22/31] Issue #10213 Ensuring proper usage of StartLog logging (to prevent accidental exceptions) (#10214) --- .../main/java/org/eclipse/jetty/start/BaseBuilder.java | 4 ++-- .../src/main/java/org/eclipse/jetty/start/FS.java | 5 +---- .../java/org/eclipse/jetty/start/FileInitializer.java | 6 +----- .../src/main/java/org/eclipse/jetty/start/Main.java | 10 +++++----- .../main/java/org/eclipse/jetty/start/PathFinder.java | 10 +++++----- .../java/org/eclipse/jetty/start/PathMatchers.java | 6 +++--- .../main/java/org/eclipse/jetty/start/StartArgs.java | 8 +++++--- .../java/org/eclipse/jetty/start/StartEnvironment.java | 4 ++-- .../eclipse/jetty/start/builders/StartDirBuilder.java | 2 +- .../jetty/start/fileinits/BaseHomeFileInitializer.java | 4 ++-- .../jetty/start/fileinits/DownloadFileInitializer.java | 2 +- .../jetty/start/fileinits/LocalFileInitializer.java | 2 +- .../start/fileinits/MavenLocalRepoFileInitializer.java | 4 ++-- 13 files changed, 31 insertions(+), 36 deletions(-) diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java index b0a460e81e4..2ab2ec06f6e 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java @@ -207,7 +207,7 @@ public class BaseBuilder List startLines = new ArrayList<>(); for (Path path : paths) { - StartLog.info("copy " + baseHome.toShortForm(path) + " into " + baseHome.toShortForm(startini)); + StartLog.info("copy %s into %s", baseHome.toShortForm(path), baseHome.toShortForm(startini)); startLines.add(""); startLines.add("# Config from " + baseHome.toShortForm(path)); startLines.addAll(Files.readAllLines(path)); @@ -250,7 +250,7 @@ public class BaseBuilder if (FS.ensureDirectoryExists(startd)) { - StartLog.info("mkdir " + baseHome.toShortForm(startd)); + StartLog.info("mkdir %s", baseHome.toShortForm(startd)); modified.set(true); } diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java index 559161a8ebe..a1608d48f8c 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FS.java @@ -23,12 +23,9 @@ import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.FileTime; -import java.util.HashMap; import java.util.Iterator; -import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jetty.util.FileID; @@ -130,7 +127,7 @@ public class FS if (!Files.isDirectory(path)) { // not a directory (as expected) - StartLog.warn("Not a directory: " + path); + StartLog.warn("Not a directory: %s", path); return false; } diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java index 0d26cdcf894..7dd9a2b788a 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/FileInitializer.java @@ -14,14 +14,10 @@ package org.eclipse.jetty.start; import java.io.IOException; -import java.io.InputStream; -import java.net.HttpURLConnection; import java.net.URI; -import java.net.URLConnection; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardCopyOption; import java.util.HashSet; import java.util.Set; @@ -167,7 +163,7 @@ public abstract class FileInitializer { if (FS.ensureDirectoryExists(to)) { - StartLog.info("mkdir " + _basehome.toShortForm(to)); + StartLog.info("mkdir %s", _basehome.toShortForm(to)); modified = true; } diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java index 4ccc2325ac3..c7bf741455a 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java @@ -186,8 +186,8 @@ public class Main StartLog.error("Do not start with ${jetty.base} == ${jetty.home}!"); else StartLog.error("No enabled jetty modules found!"); - StartLog.info("${jetty.home} = " + getBaseHome().getHomePath()); - StartLog.info("${jetty.base} = " + getBaseHome().getBasePath()); + StartLog.info("${jetty.home} = %s", getBaseHome().getHomePath()); + StartLog.info("${jetty.base} = %s", getBaseHome().getBasePath()); StartLog.error("Please create and/or configure a ${jetty.base} directory."); usageExit(ERR_INVOKE_MAIN); return; @@ -201,7 +201,7 @@ public class Main } catch (ClassNotFoundException e) { - StartLog.error("Unable to find: " + mainclass); + StartLog.error("Unable to find: %s", mainclass); StartLog.debug(e); usageExit(ERR_INVOKE_MAIN); return; @@ -489,7 +489,7 @@ public class Main final Process process = pbuilder.start(); Runtime.getRuntime().addShutdownHook(new Thread(() -> { - StartLog.debug("Destroying " + process); + StartLog.debug("Destroying %s", process); process.destroy(); })); @@ -685,7 +685,7 @@ public class Main } else { - StartLog.warn("Unable to find resource: " + resourceName); + StartLog.warn("Unable to find resource: %s", resourceName); } } catch (IOException e) diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathFinder.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathFinder.java index c9fb2c16f15..a139e142914 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathFinder.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathFinder.java @@ -42,7 +42,7 @@ public class PathFinder extends SimpleFileVisitor private void addHit(Path path) { String relPath = basePath.relativize(path).toString(); - StartLog.debug("Found [" + relPath + "] " + path); + StartLog.debug("Found [%s] %s", relPath, path); hits.put(relPath, path); } @@ -76,7 +76,7 @@ public class PathFinder extends SimpleFileVisitor { if (dirMatcher.matches(dir)) { - StartLog.trace("Following dir: " + dir); + StartLog.trace("Following dir: %s", dir); if (includeDirsInResults && fileMatcher.matches(dir)) { addHit(dir); @@ -85,7 +85,7 @@ public class PathFinder extends SimpleFileVisitor } else { - StartLog.trace("Skipping dir: " + dir); + StartLog.trace("Skipping dir: %s", dir); return FileVisitResult.SKIP_SUBTREE; } } @@ -131,7 +131,7 @@ public class PathFinder extends SimpleFileVisitor } else { - StartLog.trace("Ignoring file: " + file); + StartLog.trace("Ignoring file: %s", file); } return FileVisitResult.CONTINUE; } @@ -143,7 +143,7 @@ public class PathFinder extends SimpleFileVisitor { if (!NOTIFIED_PATHS.contains(file)) { - StartLog.warn("skipping detected filesystem loop: " + file); + StartLog.warn("skipping detected filesystem loop: %s", file); NOTIFIED_PATHS.add(file); } return FileVisitResult.SKIP_SUBTREE; diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathMatchers.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathMatchers.java index 27766f6d19c..437fbcd06cb 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathMatchers.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/PathMatchers.java @@ -84,7 +84,7 @@ public class PathMatchers // use FileSystem default pattern behavior if (pattern.startsWith("glob:") || pattern.startsWith("regex:")) { - StartLog.debug("Using Standard " + fs.getClass().getName() + " pattern: " + pattern); + StartLog.debug("Using Standard %s pattern: %s", fs.getClass().getName(), pattern); return fs.getPathMatcher(pattern); } @@ -93,14 +93,14 @@ public class PathMatchers if (isAbsolute(pattern)) { String pat = "glob:" + pattern; - StartLog.debug("Using absolute path pattern: " + pat); + StartLog.debug("Using absolute path pattern: %s", pat); return fs.getPathMatcher(pat); } // Doesn't start with filesystem root, then assume the pattern // is a relative file path pattern. String pat = "glob:**/" + pattern; - StartLog.debug("Using relative path pattern: " + pat); + StartLog.debug("Using relative path pattern: %s", pat); return fs.getPathMatcher(pat); } diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java index 8e6e255c171..30d73fa2f08 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java @@ -428,9 +428,9 @@ public class StartArgs for (String rawlibref : module.getLibs()) { - StartLog.debug("rawlibref = " + rawlibref); + StartLog.debug("rawlibref = %s", rawlibref); String libref = environment.getProperties().expand(rawlibref); - StartLog.debug("expanded = " + libref); + StartLog.debug("expanded = %s", libref); for (Path libpath : baseHome.getPaths(libref)) { @@ -555,6 +555,7 @@ public class StartArgs if (parts.contains("path")) { Classpath classpath = jettyEnvironment.getClasspath(); + StartLog.debug("classpath=%s - isJPMS=%b", classpath, isJPMS()); if (isJPMS()) { Map> dirsAndFiles = StreamSupport.stream(classpath.spliterator(), false) @@ -605,6 +606,7 @@ public class StartArgs } generateJpmsArgs(cmd); + StartLog.debug("JPMS resulting cmd=%s", cmd.toCommandLine()); } else if (!classpath.isEmpty()) { @@ -1255,7 +1257,7 @@ public class StartArgs if (arg.startsWith("--add-to-start=") || arg.startsWith("--add-to-startd=")) { String value = Props.getValue(arg); - StartLog.warn("Option " + arg.split("=")[0] + " is deprecated! Instead use: --add-modules=%s", value); + StartLog.warn("Option %s is deprecated! Instead use: --add-modules=%s", arg.split("=")[0], value); } startModules.addAll(Props.getValues(arg)); run = false; diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartEnvironment.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartEnvironment.java index 6c328cdf595..a0ee205ca13 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartEnvironment.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/StartEnvironment.java @@ -231,9 +231,9 @@ public class StartEnvironment StartLog.debug("Expanding Libs"); for (String rawlibref : _libRefs) { - StartLog.debug("rawlibref = " + rawlibref); + StartLog.debug("rawlibref = %s", rawlibref); String libref = getProperties().expand(rawlibref); - StartLog.debug("expanded = " + libref); + StartLog.debug("expanded = %s", libref); // perform path escaping (needed by windows) libref = libref.replaceAll("\\\\([^\\\\])", "\\\\\\\\$1"); diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/builders/StartDirBuilder.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/builders/StartDirBuilder.java index 528c77921d3..909ab543e2d 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/builders/StartDirBuilder.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/builders/StartDirBuilder.java @@ -42,7 +42,7 @@ public class StartDirBuilder implements BaseBuilder.Config this.baseHome = baseBuilder.getBaseHome(); this.startDir = baseHome.getBasePath("start.d"); if (FS.ensureDirectoryExists(startDir)) - StartLog.info("mkdir " + baseHome.toShortForm(startDir)); + StartLog.info("mkdir %s", baseHome.toShortForm(startDir)); } @Override diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/BaseHomeFileInitializer.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/BaseHomeFileInitializer.java index 202e228e921..55a5eb59e54 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/BaseHomeFileInitializer.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/BaseHomeFileInitializer.java @@ -64,7 +64,7 @@ public class BaseHomeFileInitializer extends FileInitializer else if (FS.ensureDirectoryExists(destination)) { modified = true; - StartLog.info("mkdir " + _basehome.toShortForm(destination)); + StartLog.info("mkdir %s", _basehome.toShortForm(destination)); } copyDirectory(source, destination); @@ -74,7 +74,7 @@ public class BaseHomeFileInitializer extends FileInitializer if (FS.ensureDirectoryExists(destination.getParent())) { modified = true; - StartLog.info("mkdir " + _basehome.toShortForm(destination.getParent())); + StartLog.info("mkdir %s", _basehome.toShortForm(destination.getParent())); } if (!FS.exists(destination)) diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java index c6aca076d95..738c7999b77 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/DownloadFileInitializer.java @@ -61,7 +61,7 @@ public abstract class DownloadFileInitializer extends FileInitializer } if (FS.ensureDirectoryExists(destination.getParent())) - StartLog.info("mkdir " + _basehome.toShortForm(destination.getParent())); + StartLog.info("mkdir %s", _basehome.toShortForm(destination.getParent())); StartLog.info("download %s to %s", uri, _basehome.toShortForm(destination)); diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/LocalFileInitializer.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/LocalFileInitializer.java index de084099132..9769d8cf8bc 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/LocalFileInitializer.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/LocalFileInitializer.java @@ -73,7 +73,7 @@ public class LocalFileInitializer extends FileInitializer // Create directory boolean mkdir = FS.ensureDirectoryExists(destination); if (mkdir) - StartLog.info("mkdir " + _basehome.toShortForm(destination)); + StartLog.info("mkdir %s", _basehome.toShortForm(destination)); return mkdir; } diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java index a9f1c3c4e12..cbd8674dadd 100644 --- a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java +++ b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/fileinits/MavenLocalRepoFileInitializer.java @@ -165,7 +165,7 @@ public class MavenLocalRepoFileInitializer extends DownloadFileInitializer if (!FS.canReadFile(localFile)) { if (FS.ensureDirectoryExists(localFile.getParent())) - StartLog.info("mkdir " + _basehome.toShortForm(localFile.getParent())); + StartLog.info("mkdir %s", _basehome.toShortForm(localFile.getParent())); download(coords, localFile); if (!FS.canReadFile(localFile)) { @@ -209,7 +209,7 @@ public class MavenLocalRepoFileInitializer extends DownloadFileInitializer if (localRepoFile != null) { if (FS.ensureDirectoryExists(destination.getParent())) - StartLog.info("mkdir " + _basehome.toShortForm(destination.getParent())); + StartLog.info("mkdir %s", _basehome.toShortForm(destination.getParent())); StartLog.info("copy %s to %s", localRepoFile, _basehome.toShortForm(destination)); Files.copy(localRepoFile, destination); return true; From 5aea1e44b7c3d41e08c5b78e28e6b2ebb70d6213 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 18 Aug 2023 00:54:05 +1000 Subject: [PATCH 23/31] Experiment/12/improve default servlet (#10222) * Improve Jetty 12 DefaultServlet + don't wrap the httpServletRequest unless necessary due to wrapping + don't wrap the httpServletResponse unless necessary due to wrapping + send content asynchronously if large and unfiltered + Remove unused boolean return from ServletChannel.handle + added TODOs where range request handling could calculate content length + Call multipartlength, even though it is always -1 + Use static for bytes written --- .../org/eclipse/jetty/http/HttpGenerator.java | 2 +- .../org/eclipse/jetty/http/MultiPart.java | 2 +- .../internal/DeferredAuthenticationState.java | 6 + .../org/eclipse/jetty/server/Request.java | 10 +- .../eclipse/jetty/server/ResourceService.java | 9 +- .../org/eclipse/jetty/server/Response.java | 23 +- .../handler/ReHandlingErrorHandler.java | 3 +- .../handler/gzip/GzipResponseAndCallback.java | 8 +- .../server/internal/HttpChannelState.java | 27 +- .../java/org/eclipse/jetty/util/Callback.java | 3 +- .../org/eclipse/jetty/util/ExceptionUtil.java | 13 + .../jetty/ee10/servlet/AsyncContextState.java | 5 - .../jetty/ee10/servlet/DefaultServlet.java | 646 ++++-------------- .../jetty/ee10/servlet/Dispatcher.java | 28 +- .../jetty/ee10/servlet/HttpOutput.java | 17 +- .../jetty/ee10/servlet/ServletApiRequest.java | 1 - .../jetty/ee10/servlet/ServletChannel.java | 30 +- .../ee10/servlet/ServletChannelState.java | 2 +- .../ee10/servlet/ServletCoreRequest.java | 261 +++++++ .../ee10/servlet/ServletCoreResponse.java | 379 ++++++++++ .../ee10/servlet/DefaultServletTest.java | 14 +- .../eclipse/jetty/ee9/nested/HttpOutput.java | 3 +- .../jetty/ee9/nested/ResponseTest.java | 6 + 23 files changed, 902 insertions(+), 596 deletions(-) create mode 100644 jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreRequest.java create mode 100644 jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreResponse.java diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java index f173ff2e87f..2b88b73a919 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java @@ -774,7 +774,7 @@ public class HttpGenerator } if (LOG.isDebugEnabled()) - LOG.debug(_endOfContent.toString()); + LOG.debug("endOfContent {} content-Length {}", _endOfContent.toString(), contentLength); // Add transfer encoding if it is not chunking if (transferEncoding != null) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index 14ec49ea3fd..064f343e063 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java @@ -656,7 +656,7 @@ public class MultiPart @Override public long getLength() { - // TODO: it is difficult to calculate the length because + // TODO: #10307 it is difficult to calculate the length because // we need to allow for customization of the headers from // subclasses, and then serialize all the headers to get // their length (handling UTF-8 values) and we don't want diff --git a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/internal/DeferredAuthenticationState.java b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/internal/DeferredAuthenticationState.java index 2e37313241b..3309f2f6da4 100644 --- a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/internal/DeferredAuthenticationState.java +++ b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/internal/DeferredAuthenticationState.java @@ -181,6 +181,12 @@ public class DeferredAuthenticationState implements AuthenticationState.Deferred return true; } + @Override + public boolean hasLastWrite() + { + return false; + } + @Override public boolean isCompletedSuccessfully() { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index e84dad2ca79..c9ff22c9597 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -768,13 +768,13 @@ public interface Request extends Attributes, Content.Source } @SuppressWarnings("unchecked") - static T as(Request request, Class type) + static T as(Request request, Class type) { - while (request instanceof Request.Wrapper wrapper) + while (request != null) { - if (type.isInstance(wrapper)) - return (T)wrapper; - request = wrapper.getWrapped(); + if (type.isInstance(request)) + return (T)request; + request = request instanceof Request.Wrapper wrapper ? wrapper.getWrapped() : null; } return null; } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index b737459b939..1024b2e5455 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -145,7 +145,7 @@ public class ResourceService return _gzipEquivalentFileExtensions; } - public void doGet(Request request, Response response, Callback callback, HttpContent content) throws Exception + public void doGet(Request request, Response response, Callback callback, HttpContent content) { String pathInContext = Request.getPathInContext(request); @@ -523,7 +523,7 @@ public class ResourceService // TODO : check conditional headers. serveWelcome(request, response, callback, welcomeAction.target); case REHANDLE -> rehandleWelcome(request, response, callback, welcomeAction.target); - }; + } } /** @@ -683,14 +683,15 @@ public class ResourceService } // There are multiple non-overlapping ranges, send a multipart/byteranges 206 response. - putHeaders(response, content, NO_CONTENT_LENGTH); response.setStatus(HttpStatus.PARTIAL_CONTENT_206); String contentType = "multipart/byteranges; boundary="; String boundary = MultiPart.generateBoundary(null, 24); - response.getHeaders().put(HttpHeader.CONTENT_TYPE, contentType + boundary); MultiPartByteRanges.ContentSource byteRanges = new MultiPartByteRanges.ContentSource(boundary); ranges.forEach(range -> byteRanges.addPart(new MultiPartByteRanges.Part(content.getContentTypeValue(), content.getResource().getPath(), range, contentLength))); byteRanges.close(); + long partsContentLength = byteRanges.getLength(); + putHeaders(response, content, partsContentLength); + response.getHeaders().put(HttpHeader.CONTENT_TYPE, contentType + boundary); Content.copy(byteRanges, response, callback); } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index ab71311d89a..c1eedf615a1 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -99,6 +99,13 @@ public interface Response extends Content.Sink */ boolean isCommitted(); + /** + *

Returns whether the last write has been initiated on the response.

+ * + * @return {@code true} if {@code last==true} has been passed to {@link #write(boolean, ByteBuffer, Callback)}. + */ + boolean hasLastWrite(); + /** *

Returns whether the response completed successfully.

*

The response HTTP status code, HTTP headers and content @@ -207,13 +214,13 @@ public interface Response extends Content.Sink * @see Wrapper */ @SuppressWarnings("unchecked") - static T as(Response response, Class type) + static T as(Response response, Class type) { - while (response instanceof Response.Wrapper wrapper) + while (response != null) { - if (type.isInstance(wrapper)) - return (T)wrapper; - response = wrapper.getWrapped(); + if (type.isInstance(response)) + return (T)response; + response = response instanceof Response.Wrapper wrapper ? wrapper.getWrapped() : null; } return null; } @@ -580,6 +587,12 @@ public interface Response extends Content.Sink return getWrapped().isCommitted(); } + @Override + public boolean hasLastWrite() + { + return getWrapped().hasLastWrite(); + } + @Override public boolean isCompletedSuccessfully() { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ReHandlingErrorHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ReHandlingErrorHandler.java index 9c87501b056..46868279f90 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ReHandlingErrorHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ReHandlingErrorHandler.java @@ -69,8 +69,7 @@ public abstract class ReHandlingErrorHandler extends ErrorHandler { if (LOG.isDebugEnabled()) LOG.debug("Unable to process error {}", reRequest, e); - if (ExceptionUtil.areNotAssociated(cause, e)) - cause.addSuppressed(e); + ExceptionUtil.addSuppressedIfNotAssociated(cause, e); response.setStatus(code); } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipResponseAndCallback.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipResponseAndCallback.java index bd311d157cc..f8ff65f26ab 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipResponseAndCallback.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipResponseAndCallback.java @@ -136,7 +136,13 @@ public class GzipResponseAndCallback extends Response.Wrapper implements Callbac case NOT_COMPRESSING -> super.write(last, content, callback); case COMMITTING -> callback.failed(new WritePendingException()); case COMPRESSING -> gzip(last, callback, content); - default -> callback.failed(new IllegalStateException("state=" + _state.get())); + default -> + { + if (BufferUtil.isEmpty(content)) + callback.succeeded(); + else + callback.failed(new IllegalStateException("state=" + _state.get())); + } } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java index 8e119a838f4..3fc69037626 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java @@ -473,8 +473,7 @@ public class HttpChannelState implements HttpChannel, Components } catch (Throwable throwable) { - if (ExceptionUtil.areNotAssociated(x, throwable)) - x.addSuppressed(throwable); + ExceptionUtil.addSuppressedIfNotAssociated(x, throwable); } // If the application has not been otherwise informed of the failure @@ -1080,8 +1079,7 @@ public class HttpChannelState implements HttpChannel, Components } catch (Throwable t) { - if (ExceptionUtil.areNotAssociated(throwable, t)) - throwable.addSuppressed(t); + ExceptionUtil.addSuppressedIfNotAssociated(throwable, t); } finally { @@ -1354,6 +1352,18 @@ public class HttpChannelState implements HttpChannel, Components return _httpFields.isCommitted(); } + @Override + public boolean hasLastWrite() + { + try (AutoLock ignored = _request._lock.lock()) + { + if (_request._httpChannelState == null) + return true; + + return _request._httpChannelState._streamSendState != StreamSendState.SENDING; + } + } + @Override public boolean isCompletedSuccessfully() { @@ -1540,8 +1550,7 @@ public class HttpChannelState implements HttpChannel, Components // Consume any input. Throwable unconsumed = stream.consumeAvailable(); - if (ExceptionUtil.areNotAssociated(unconsumed, failure)) - failure.addSuppressed(unconsumed); + ExceptionUtil.addSuppressedIfNotAssociated(failure, unconsumed); if (LOG.isDebugEnabled()) LOG.debug("failed stream.isCommitted={}, response.isCommitted={} {}", httpChannelState._stream.isCommitted(), httpChannelState._response.isCommitted(), this); @@ -1689,8 +1698,7 @@ public class HttpChannelState implements HttpChannel, Components Callback.from(() -> httpChannelState._handlerInvoker.failed(failure), x -> { - if (ExceptionUtil.areNotAssociated(failure, x)) - failure.addSuppressed(x); + ExceptionUtil.addSuppressedIfNotAssociated(failure, x); httpChannelState._handlerInvoker.failed(failure); })); } @@ -1758,8 +1766,7 @@ public class HttpChannelState implements HttpChannel, Components } catch (Throwable t) { - if (ExceptionUtil.areNotAssociated(failure, t)) - failure.addSuppressed(t); + ExceptionUtil.addSuppressedIfNotAssociated(failure, t); super.onError(task, failure); } } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Callback.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Callback.java index 11ff341e555..22cb55ee241 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Callback.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Callback.java @@ -478,8 +478,7 @@ public interface Callback extends Invocable } catch (Throwable t) { - if (ExceptionUtil.areNotAssociated(x, t)) - x.addSuppressed(t); + ExceptionUtil.addSuppressedIfNotAssociated(x, t); } finally { diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ExceptionUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ExceptionUtil.java index 279e5136686..069e844da71 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ExceptionUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ExceptionUtil.java @@ -26,6 +26,7 @@ import org.eclipse.jetty.util.thread.Invocable; */ public class ExceptionUtil { + /** *

Convert a {@link Throwable} to a specific type by casting or construction on a new instance.

* @@ -178,6 +179,18 @@ public class ExceptionUtil return true; } + /** + * Add a suppressed exception if it is not associated. + * @see #areNotAssociated(Throwable, Throwable) + * @param throwable The main Throwable + * @param suppressed The Throwable to suppress if it is not associated. + */ + public static void addSuppressedIfNotAssociated(Throwable throwable, Throwable suppressed) + { + if (areNotAssociated(throwable, suppressed)) + throwable.addSuppressed(suppressed); + } + /** * Decorate a Throwable with the suppressed errors and return it. * @param t the throwable diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextState.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextState.java index d3c6d8aecf9..cc2513a31c7 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextState.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextState.java @@ -149,11 +149,6 @@ public class AsyncContextState implements AsyncContext _state = null; } - public ServletChannelState getServletChannelState() - { - return state(); - } - public static class WrappedAsyncListener implements AsyncListener { private final AsyncListener _listener; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java index 729e84e17ad..ff8ba93a069 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/DefaultServlet.java @@ -15,26 +15,17 @@ package org.eclipse.jetty.ee10.servlet; import java.io.FileNotFoundException; import java.io.IOException; -import java.io.InputStreamReader; import java.net.URI; -import java.nio.ByteBuffer; import java.nio.file.InvalidPathException; import java.time.Duration; import java.util.ArrayList; -import java.util.EnumSet; -import java.util.Enumeration; -import java.util.HashSet; import java.util.List; -import java.util.ListIterator; import java.util.Locale; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.StringTokenizer; -import java.util.concurrent.CompletableFuture; -import java.util.function.Supplier; -import java.util.stream.Collectors; +import jakarta.servlet.AsyncContext; import jakarta.servlet.DispatcherType; import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletContext; @@ -43,16 +34,12 @@ import jakarta.servlet.UnavailableException; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import jakarta.servlet.http.HttpServletResponseWrapper; import jakarta.servlet.http.MappingMatch; import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.HttpException; import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.content.FileMappingHttpContentFactory; import org.eclipse.jetty.http.content.HttpContent; @@ -60,7 +47,6 @@ import org.eclipse.jetty.http.content.PreCompressedHttpContentFactory; import org.eclipse.jetty.http.content.ResourceHttpContentFactory; import org.eclipse.jetty.http.content.ValidatingCachingHttpContentFactory; import org.eclipse.jetty.http.content.VirtualHttpContentFactory; -import org.eclipse.jetty.io.ByteBufferInputStream; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.Request; @@ -69,9 +55,8 @@ import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.Blocker; -import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; -import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.ExceptionUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; @@ -79,6 +64,8 @@ import org.eclipse.jetty.util.resource.Resources; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.eclipse.jetty.util.URIUtil.encodePath; + /** *

The default Servlet, normally mapped to {@code /}, that handles static resources.

*

The following init parameters are supported:

@@ -185,7 +172,6 @@ public class DefaultServlet extends HttpServlet private ServletContextHandler _contextHandler; private ServletResourceService _resourceService; private WelcomeServletMode _welcomeServletMode; - private Resource _baseResource; public ResourceService getResourceService() { @@ -198,14 +184,14 @@ public class DefaultServlet extends HttpServlet _contextHandler = initContextHandler(getServletContext()); _resourceService = new ServletResourceService(_contextHandler); _resourceService.setWelcomeFactory(_resourceService); - _baseResource = _contextHandler.getBaseResource(); + Resource baseResource = _contextHandler.getBaseResource(); String rb = getInitParameter("baseResource", "resourceBase"); if (rb != null) { try { - _baseResource = Objects.requireNonNull(_contextHandler.newResource(rb)); + baseResource = Objects.requireNonNull(_contextHandler.newResource(rb)); } catch (Exception e) { @@ -222,7 +208,7 @@ public class DefaultServlet extends HttpServlet if (contentFactory == null) { MimeTypes mimeTypes = _contextHandler.getMimeTypes(); - ResourceFactory resourceFactory = _baseResource != null ? ResourceFactory.of(_baseResource) : this::getResource; + ResourceFactory resourceFactory = baseResource != null ? ResourceFactory.of(baseResource) : this::getResource; contentFactory = new ResourceHttpContentFactory(resourceFactory, mimeTypes); // Use the servers default stylesheet unless there is one explicitly set by an init param. @@ -326,7 +312,7 @@ public class DefaultServlet extends HttpServlet if (LOG.isDebugEnabled()) { - LOG.debug(" .baseResource = {}", _baseResource); + LOG.debug(" .baseResource = {}", baseResource); LOG.debug(" .resourceService = {}", _resourceService); LOG.debug(" .welcomeServletMode = {}", _welcomeServletMode); } @@ -458,18 +444,18 @@ public class DefaultServlet extends HttpServlet } @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void doGet(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) throws ServletException, IOException { - String includedServletPath = (String)req.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); - String encodedPathInContext = getEncodedPathInContext(req, includedServletPath); + String includedServletPath = (String)httpServletRequest.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); + String encodedPathInContext = getEncodedPathInContext(httpServletRequest, includedServletPath); boolean included = includedServletPath != null; if (LOG.isDebugEnabled()) - LOG.debug("doGet(req={}, resp={}) pathInContext={}, included={}", req, resp, encodedPathInContext, included); + LOG.debug("doGet(hsReq={}, hsResp={}) pathInContext={}, included={}", httpServletRequest, httpServletResponse, encodedPathInContext, included); try { - HttpContent content = _resourceService.getContent(encodedPathInContext, ServletContextRequest.getServletContextRequest(req)); + HttpContent content = _resourceService.getContent(encodedPathInContext, ServletContextRequest.getServletContextRequest(httpServletRequest)); if (LOG.isDebugEnabled()) LOG.debug("content = {}", content); @@ -487,13 +473,31 @@ public class DefaultServlet extends HttpServlet } // no content - resp.sendError(404); + httpServletResponse.sendError(404); } else { - ServletCoreRequest coreRequest = new ServletCoreRequest(req); - ServletCoreResponse coreResponse = new ServletCoreResponse(coreRequest, resp, included); + // lookup the core request and response as wrapped by the ServletContextHandler + ServletContextRequest servletContextRequest = ServletContextRequest.getServletContextRequest(httpServletRequest); + ServletContextResponse servletContextResponse = servletContextRequest.getServletContextResponse(); + ServletChannel servletChannel = servletContextRequest.getServletChannel(); + // If the servlet request has not been wrapped, + // we can use the core request directly, + // otherwise wrap the servlet request as a core request + Request coreRequest = httpServletRequest instanceof ServletApiRequest + ? servletChannel.getRequest() + : new ServletCoreRequest(httpServletRequest); + + // If the servlet response has been wrapped and has been written to, + // then the servlet response must be wrapped as a core response + // otherwise we can use the core response directly. + boolean useServletResponse = !(httpServletResponse instanceof ServletApiResponse) || servletContextResponse.isWritingOrStreaming(); + Response coreResponse = useServletResponse + ? new ServletCoreResponse(coreRequest, httpServletResponse, included) + : servletChannel.getResponse(); + + // If the core response is already committed then do nothing more if (coreResponse.isCommitted()) { if (LOG.isDebugEnabled()) @@ -501,30 +505,39 @@ public class DefaultServlet extends HttpServlet return; } + // Get the content length before we may wrap the content + long contentLength = content.getContentLengthValue(); + // Servlet Filters could be interacting with the Response already. - if (coreResponse.isHttpServletResponseWrapped() || - coreResponse.isWritingOrStreaming()) - { + if (useServletResponse) content = new UnknownLengthHttpContent(content); - } - ServletContextResponse contextResponse = coreResponse.getServletContextResponse(); - if (contextResponse != null) - { - String characterEncoding = contextResponse.getRawCharacterEncoding(); - if (characterEncoding != null) - content = new ForcedCharacterEncodingHttpContent(content, characterEncoding); - } + // The character encoding may be forced + String characterEncoding = servletContextResponse.getRawCharacterEncoding(); + if (characterEncoding != null) + content = new ForcedCharacterEncodingHttpContent(content, characterEncoding); - // serve content - try (Blocker.Callback callback = Blocker.callback()) + // If async is supported and the unwrapped content is larger than an output buffer + if (httpServletRequest.isAsyncSupported() && + (contentLength < 0 || contentLength > coreRequest.getConnectionMetaData().getHttpConfiguration().getOutputBufferSize())) { + // send the content asynchronously + AsyncContext asyncContext = httpServletRequest.startAsync(); + Callback callback = new AsyncContextCallback(asyncContext, httpServletResponse); _resourceService.doGet(coreRequest, coreResponse, callback, content); - callback.block(); } - catch (Exception e) + else { - throw new ServletException(e); + // send the content blocking + try (Blocker.Callback callback = Blocker.callback()) + { + _resourceService.doGet(coreRequest, coreResponse, callback, content); + callback.block(); + } + catch (Exception e) + { + throw new ServletException(e); + } } } } @@ -534,16 +547,16 @@ public class DefaultServlet extends HttpServlet LOG.debug("InvalidPathException for pathInContext: {}", encodedPathInContext, e); if (included) throw new FileNotFoundException(encodedPathInContext); - resp.setStatus(404); + httpServletResponse.setStatus(404); } } protected String getEncodedPathInContext(HttpServletRequest req, String includedServletPath) { if (includedServletPath != null) - return URIUtil.encodePath(getIncludedPathInContext(req, includedServletPath, !isDefaultMapping(req))); + return encodePath(getIncludedPathInContext(req, includedServletPath, !isDefaultMapping(req))); else if (!isDefaultMapping(req)) - return URIUtil.encodePath(req.getPathInfo()); + return encodePath(req.getPathInfo()); else if (req instanceof ServletApiRequest apiRequest) return Context.getPathInContext(req.getContextPath(), apiRequest.getRequest().getHttpURI().getCanonicalPath()); else @@ -589,476 +602,6 @@ public class DefaultServlet extends HttpServlet return result; } - private static class ServletCoreRequest extends Request.Wrapper - { - // TODO fully implement this class and move it to the top level - // TODO Some methods are directed to core that probably should be intercepted - - private final HttpServletRequest _servletRequest; - private final HttpFields _httpFields; - private final HttpURI _uri; - - ServletCoreRequest(HttpServletRequest request) - { - super(ServletContextRequest.getServletContextRequest(request)); - _servletRequest = request; - - HttpFields.Mutable fields = HttpFields.build(); - - Enumeration headerNames = request.getHeaderNames(); - while (headerNames.hasMoreElements()) - { - String headerName = headerNames.nextElement(); - Enumeration headerValues = request.getHeaders(headerName); - while (headerValues.hasMoreElements()) - { - String headerValue = headerValues.nextElement(); - fields.add(new HttpField(headerName, headerValue)); - } - } - - _httpFields = fields.asImmutable(); - String includedServletPath = (String)request.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); - boolean included = includedServletPath != null; - if (request.getDispatcherType() == DispatcherType.REQUEST) - _uri = getWrapped().getHttpURI(); - else if (included) - _uri = Request.newHttpURIFrom(getWrapped(), URIUtil.encodePath(getIncludedPathInContext(request, includedServletPath, false))); - else - _uri = Request.newHttpURIFrom(getWrapped(), URIUtil.encodePath(URIUtil.addPaths(_servletRequest.getServletPath(), _servletRequest.getPathInfo()))); - } - - @Override - public HttpFields getHeaders() - { - return _httpFields; - } - - @Override - public HttpURI getHttpURI() - { - return _uri; - } - - @Override - public String getId() - { - return _servletRequest.getRequestId(); - } - - @Override - public String getMethod() - { - return _servletRequest.getMethod(); - } - - @Override - public boolean isSecure() - { - return _servletRequest.isSecure(); - } - - @Override - public Object removeAttribute(String name) - { - Object value = _servletRequest.getAttribute(name); - _servletRequest.removeAttribute(name); - return value; - } - - @Override - public Object setAttribute(String name, Object attribute) - { - Object value = _servletRequest.getAttribute(name); - _servletRequest.setAttribute(name, attribute); - return value; - } - - @Override - public Object getAttribute(String name) - { - return _servletRequest.getAttribute(name); - } - - @Override - public Set getAttributeNameSet() - { - Set set = new HashSet<>(); - Enumeration e = _servletRequest.getAttributeNames(); - while (e.hasMoreElements()) - set.add(e.nextElement()); - return set; - } - - @Override - public void clearAttributes() - { - Enumeration e = _servletRequest.getAttributeNames(); - while (e.hasMoreElements()) - _servletRequest.removeAttribute(e.nextElement()); - } - } - - private static class HttpServletResponseHttpFields implements HttpFields.Mutable - { - private final HttpServletResponse _response; - - private HttpServletResponseHttpFields(HttpServletResponse response) - { - _response = response; - } - - @Override - public ListIterator listIterator() - { - // The minimum requirement is to implement the listIterator, but it is inefficient. - // Other methods are implemented for efficiency. - final ListIterator list = _response.getHeaderNames().stream() - .map(n -> new HttpField(n, _response.getHeader(n))) - .collect(Collectors.toList()) - .listIterator(); - - return new ListIterator<>() - { - HttpField _last; - - @Override - public boolean hasNext() - { - return list.hasNext(); - } - - @Override - public HttpField next() - { - return _last = list.next(); - } - - @Override - public boolean hasPrevious() - { - return list.hasPrevious(); - } - - @Override - public HttpField previous() - { - return _last = list.previous(); - } - - @Override - public int nextIndex() - { - return list.nextIndex(); - } - - @Override - public int previousIndex() - { - return list.previousIndex(); - } - - @Override - public void remove() - { - if (_last != null) - { - // This is not exactly the right semantic for repeated field names - list.remove(); - _response.setHeader(_last.getName(), null); - } - } - - @Override - public void set(HttpField httpField) - { - list.set(httpField); - _response.setHeader(httpField.getName(), httpField.getValue()); - } - - @Override - public void add(HttpField httpField) - { - list.add(httpField); - _response.addHeader(httpField.getName(), httpField.getValue()); - } - }; - } - - @Override - public Mutable add(String name, String value) - { - _response.addHeader(name, value); - return this; - } - - @Override - public Mutable add(HttpHeader header, HttpHeaderValue value) - { - _response.addHeader(header.asString(), value.asString()); - return this; - } - - @Override - public Mutable add(HttpHeader header, String value) - { - _response.addHeader(header.asString(), value); - return this; - } - - @Override - public Mutable add(HttpField field) - { - _response.addHeader(field.getName(), field.getValue()); - return this; - } - - @Override - public Mutable put(HttpField field) - { - _response.setHeader(field.getName(), field.getValue()); - return this; - } - - @Override - public Mutable put(String name, String value) - { - _response.setHeader(name, value); - return this; - } - - @Override - public Mutable put(HttpHeader header, HttpHeaderValue value) - { - _response.setHeader(header.asString(), value.asString()); - return this; - } - - @Override - public Mutable put(HttpHeader header, String value) - { - _response.setHeader(header.asString(), value); - return this; - } - - @Override - public Mutable put(String name, List list) - { - Objects.requireNonNull(name); - Objects.requireNonNull(list); - boolean first = true; - for (String s : list) - { - if (first) - _response.setHeader(name, s); - else - _response.addHeader(name, s); - first = false; - } - return this; - } - - @Override - public Mutable remove(HttpHeader header) - { - _response.setHeader(header.asString(), null); - return this; - } - - @Override - public Mutable remove(EnumSet fields) - { - for (HttpHeader header : fields) - remove(header); - return this; - } - - @Override - public Mutable remove(String name) - { - _response.setHeader(name, null); - return this; - } - } - - private static class ServletCoreResponse implements Response - { - // TODO fully implement this class and move it to the top level - - private final HttpServletResponse _response; - private final ServletCoreRequest _coreRequest; - private final Response _coreResponse; - private final HttpFields.Mutable _httpFields; - private final boolean _included; - - public ServletCoreResponse(ServletCoreRequest coreRequest, HttpServletResponse response, boolean included) - { - _coreRequest = coreRequest; - _response = response; - _coreResponse = ServletContextResponse.getServletContextResponse(response); - HttpFields.Mutable fields = new HttpServletResponseHttpFields(response); - if (included) - { - // If included, accept but ignore mutations. - fields = new HttpFields.Mutable.Wrapper(fields) - { - @Override - public HttpField onAddField(HttpField field) - { - return null; - } - - @Override - public boolean onRemoveField(HttpField field) - { - return false; - } - }; - } - _httpFields = fields; - _included = included; - } - - @Override - public HttpFields.Mutable getHeaders() - { - return _httpFields; - } - - public ServletContextResponse getServletContextResponse() - { - return ServletContextResponse.getServletContextResponse(_response); - } - - @Override - public boolean isCommitted() - { - return _response.isCommitted(); - } - - /** - * Test if the HttpServletResponse is wrapped by the webapp. - * - * @return true if wrapped. - */ - public boolean isHttpServletResponseWrapped() - { - return (_response instanceof HttpServletResponseWrapper); - } - - /** - * Test if {@link HttpServletResponse#getOutputStream()} or - * {@link HttpServletResponse#getWriter()} has been called already - * - * @return true if {@link HttpServletResponse} has started to write or stream content - */ - public boolean isWritingOrStreaming() - { - ServletContextResponse servletContextResponse = Response.as(_coreResponse, ServletContextResponse.class); - return servletContextResponse.isWritingOrStreaming(); - } - - public boolean isWriting() - { - ServletContextResponse servletContextResponse = Response.as(_coreResponse, ServletContextResponse.class); - return servletContextResponse.isWriting(); - } - - @Override - public void write(boolean last, ByteBuffer byteBuffer, Callback callback) - { - if (_included) - last = false; - try - { - if (BufferUtil.hasContent(byteBuffer)) - { - if (isWriting()) - { - String characterEncoding = _response.getCharacterEncoding(); - try (ByteBufferInputStream bbis = new ByteBufferInputStream(byteBuffer); - InputStreamReader reader = new InputStreamReader(bbis, characterEncoding)) - { - IO.copy(reader, _response.getWriter()); - } - - if (last) - _response.getWriter().close(); - } - else - { - BufferUtil.writeTo(byteBuffer, _response.getOutputStream()); - if (last) - _response.getOutputStream().close(); - } - } - - callback.succeeded(); - } - catch (Throwable t) - { - callback.failed(t); - } - } - - @Override - public Request getRequest() - { - return _coreRequest; - } - - @Override - public int getStatus() - { - return _response.getStatus(); - } - - @Override - public void setStatus(int code) - { - if (LOG.isDebugEnabled()) - LOG.debug("{}.setStatus({})", this.getClass().getSimpleName(), code); - if (_included) - return; - _response.setStatus(code); - } - - @Override - public Supplier getTrailersSupplier() - { - return null; - } - - @Override - public void setTrailersSupplier(Supplier trailers) - { - } - - @Override - public boolean isCompletedSuccessfully() - { - return _coreResponse.isCompletedSuccessfully(); - } - - @Override - public void reset() - { - _response.reset(); - } - - @Override - public CompletableFuture writeInterim(int status, HttpFields headers) - { - return null; - } - - @Override - public String toString() - { - return "%s@%x{%s,%s}".formatted(this.getClass().getSimpleName(), hashCode(), this._coreRequest, _response); - } - } - private class ServletResourceService extends ResourceService implements ResourceService.WelcomeFactory { private final ServletContextHandler _servletContextHandler; @@ -1214,18 +757,32 @@ public class DefaultServlet extends HttpServlet private HttpServletRequest getServletRequest(Request request) { - // TODO, this unwrapping is fragile - return ((ServletCoreRequest)request)._servletRequest; + ServletCoreRequest servletCoreRequest = Request.as(request, ServletCoreRequest.class); + if (servletCoreRequest != null) + return servletCoreRequest.getServletRequest(); + + ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class); + if (servletContextRequest != null) + return servletContextRequest.getServletApiRequest(); + + throw new IllegalStateException("instanceof " + request.getClass()); } private HttpServletResponse getServletResponse(Response response) { - // TODO, this unwrapping is fragile - return ((ServletCoreResponse)response)._response; + ServletCoreResponse servletCoreResponse = Response.as(response, ServletCoreResponse.class); + if (servletCoreResponse != null) + return servletCoreResponse.getServletResponse(); + + ServletContextResponse servletContextResponse = Response.as(response, ServletContextResponse.class); + if (servletContextResponse != null) + return servletContextResponse.getServletApiResponse(); + + throw new IllegalStateException("instanceof " + response.getClass()); } } - private static String getIncludedPathInContext(HttpServletRequest request, String includedServletPath, boolean isPathInfoOnly) + static String getIncludedPathInContext(HttpServletRequest request, String includedServletPath, boolean isPathInfoOnly) { String servletPath = isPathInfoOnly ? "/" : includedServletPath; String pathInfo = (String)request.getAttribute(RequestDispatcher.INCLUDE_PATH_INFO); @@ -1330,4 +887,45 @@ public class DefaultServlet extends HttpServlet */ EXACT } + + private static class AsyncContextCallback implements Callback + { + private final AsyncContext _asyncContext; + private final HttpServletResponse _response; + + private AsyncContextCallback(AsyncContext asyncContext, HttpServletResponse response) + { + _asyncContext = asyncContext; + _response = response; + } + + @Override + public void succeeded() + { + _asyncContext.complete(); + } + + @Override + public void failed(Throwable x) + { + try + { + if (LOG.isDebugEnabled()) + LOG.debug("AsyncContextCallback failed {}", _asyncContext, x); + // It is known that this callback is only failed if the response is already committed, + // thus we can only abort the response here. + _response.sendError(-1); + } + catch (IOException e) + { + ExceptionUtil.addSuppressedIfNotAssociated(x, e); + } + finally + { + _asyncContext.complete(); + } + if (LOG.isDebugEnabled()) + LOG.debug("Async get failed", x); + } + } } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java index 767dc2a657a..4298fa36e5f 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.ee10.servlet; +import java.io.Closeable; import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; @@ -41,6 +42,7 @@ import org.eclipse.jetty.ee10.servlet.util.ServletOutputStreamWrapper; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.io.WriterOutputStream; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.UrlEncoded; @@ -122,16 +124,18 @@ public class Dispatcher implements RequestDispatcher _mappedServlet.handle(_servletHandler, _decodedPathInContext, new ForwardRequest(httpRequest), httpResponse); // If we are not async and not closed already, then close via the possibly wrapped response. - if (!servletContextRequest.getState().isAsync() && !servletContextRequest.getHttpOutput().isClosed()) + if (!servletContextRequest.getState().isAsync() && !servletContextRequest.getServletContextResponse().hasLastWrite()) { + Closeable closeable; try { - response.getOutputStream().close(); + closeable = response.getOutputStream(); } catch (IllegalStateException e) { - response.getWriter().close(); + closeable = response.getWriter(); } + IO.close(closeable); } } @@ -624,6 +628,24 @@ public class Dispatcher implements RequestDispatcher { // NOOP for include. } + + @Override + public void sendError(int sc, String msg) throws IOException + { + // NOOP for include. + } + + @Override + public void sendError(int sc) throws IOException + { + // NOOP for include. + } + + @Override + public void sendRedirect(String location) throws IOException + { + // NOOP for include. + } } private class AsyncRequest extends ParameterRequestWrapper diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java index 365d02605c9..d57452af7ee 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java @@ -36,6 +36,7 @@ import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.server.HttpConfiguration; +import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.Blocker; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; @@ -154,12 +155,20 @@ public class HttpOutput extends ServletOutputStream implements Runnable _commitSize = _bufferSize; } } - + + /** + * @return True if any content has been written via the {@link jakarta.servlet.http.HttpServletResponse} API. + */ public boolean isWritten() { return _written > 0; } + /** + * @return The bytes written via the {@link jakarta.servlet.http.HttpServletResponse} API. This + * may differ from the bytes reported by {@link org.eclipse.jetty.server.Response#getContentBytesWritten(Response)} + * due to buffering, compression, other interception or writes that bypass the servlet API. + */ public long getWritten() { return _written; @@ -445,6 +454,9 @@ public class HttpOutput extends ServletOutputStream implements Runnable Blocker.Callback blocker = null; try (AutoLock l = _channelState.lock()) { + if (_softClose) + return; + if (_onError != null) { if (_onError instanceof IOException) @@ -1456,8 +1468,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable } catch (Throwable t) { - if (ExceptionUtil.areNotAssociated(e, t)) - e.addSuppressed(t); + ExceptionUtil.addSuppressedIfNotAssociated(e, t); } finally { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java index 47a6d62f512..41702d09266 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java @@ -100,7 +100,6 @@ public class ServletApiRequest implements HttpServletRequest private static final Logger LOG = LoggerFactory.getLogger(ServletApiRequest.class); private final ServletContextRequest _servletContextRequest; private final ServletChannel _servletChannel; - //TODO review which fields should be in ServletContextRequest private AsyncContextState _async; private String _characterEncoding; private int _inputState = ServletContextRequest.INPUT_NONE; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java index 8c70c8b424c..e103c3b0d30 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java @@ -83,7 +83,6 @@ public class ServletChannel private Response _response; private Callback _callback; private boolean _expects100Continue; - private long _written; public ServletChannel(ServletContextHandler servletContextHandler, Request request) { @@ -218,7 +217,7 @@ public class ServletChannel public long getBytesWritten() { - return _written; + return Response.getContentBytesWritten(getServletContextResponse()); } /** @@ -449,15 +448,13 @@ public class ServletChannel _request = _servletContextRequest = null; _response = null; _callback = null; - _written = 0; _expects100Continue = false; } /** * Handle the servlet request. This is called on the initial dispatch and then again on any asynchronous events. - * @return True if the channel is ready to continue handling (ie it is not suspended) */ - public boolean handle() + public void handle() { if (LOG.isDebugEnabled()) LOG.debug("handle {} {} ", _servletContextRequest.getHttpURI(), this); @@ -553,15 +550,15 @@ public class ServletChannel // If the callback has already been completed we should continue in handle loop. // Otherwise, the callback will schedule a dispatch to handle(). if (asyncCompletion.compareAndSet(false, true)) - return false; + return; } } catch (Throwable x) { if (cause == null) cause = x; - else if (ExceptionUtil.areNotAssociated(cause, x)) - cause.addSuppressed(x); + else + ExceptionUtil.addSuppressedIfNotAssociated(cause, x); if (LOG.isDebugEnabled()) LOG.debug("Could not perform ERROR dispatch, aborting", cause); if (_state.isResponseCommitted()) @@ -577,8 +574,7 @@ public class ServletChannel } catch (Throwable t) { - if (ExceptionUtil.areNotAssociated(cause, t)) - cause.addSuppressed(t); + ExceptionUtil.addSuppressedIfNotAssociated(cause, t); abort(cause); } } @@ -617,12 +613,11 @@ public class ServletChannel ResponseUtils.ensureConsumeAvailableOrNotPersistent(_servletContextRequest, _servletContextRequest.getServletContextResponse()); } - // RFC 7230, section 3.3. - if (!_servletContextRequest.isHead() && - getServletContextResponse().getStatus() != HttpStatus.NOT_MODIFIED_304 && - getServletContextResponse().isContentIncomplete(_servletContextRequest.getHttpOutput().getWritten())) + // RFC 7230, section 3.3. We do this here so that a servlet error page can be sent. + if (!_servletContextRequest.isHead() && getServletContextResponse().getStatus() != HttpStatus.NOT_MODIFIED_304) { - if (sendErrorOrAbort("Insufficient content written")) + long written = getBytesWritten(); + if (getServletContextResponse().isContentIncomplete(written) && sendErrorOrAbort("Insufficient content written %d < %d".formatted(written, getServletContextResponse().getContentLength()))) break; } @@ -649,9 +644,6 @@ public class ServletChannel if (LOG.isDebugEnabled()) LOG.debug("!handle {} {}", action, this); - - boolean suspended = action == Action.WAIT; - return !suspended; } private void reopen() @@ -664,7 +656,7 @@ public class ServletChannel * @param message the error message. * @return true if we have sent an error, false if we have aborted. */ - public boolean sendErrorOrAbort(String message) + private boolean sendErrorOrAbort(String message) { try { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java index 0045a24566c..45362875bba 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java @@ -41,7 +41,7 @@ import static jakarta.servlet.RequestDispatcher.ERROR_SERVLET_NAME; import static jakarta.servlet.RequestDispatcher.ERROR_STATUS_CODE; /** - * Implementation of AsyncContext interface that holds the state of request-response cycle. + * holder of the state of request-response cycle. */ public class ServletChannelState { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreRequest.java new file mode 100644 index 00000000000..a2822b1e002 --- /dev/null +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreRequest.java @@ -0,0 +1,261 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.ee10.servlet; + +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Predicate; + +import jakarta.servlet.DispatcherType; +import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.http.HttpServletRequest; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Components; +import org.eclipse.jetty.server.ConnectionMetaData; +import org.eclipse.jetty.server.Context; +import org.eclipse.jetty.server.HttpStream; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Session; +import org.eclipse.jetty.server.TunnelSupport; +import org.eclipse.jetty.util.URIUtil; + +import static org.eclipse.jetty.util.URIUtil.addEncodedPaths; +import static org.eclipse.jetty.util.URIUtil.encodePath; + +/** + * Wrap a {@link jakarta.servlet.ServletRequest} as a core {@link Request}. + *

+ * Whilst similar to a {@link Request.Wrapper}, this class is not a {@code Wrapper} + * as callers should not be able to access {@link Wrapper#getWrapped()} and bypass + * the {@link jakarta.servlet.ServletRequest}. + *

+ *

+ * The current implementation does not support any read operations. + *

+ */ +class ServletCoreRequest implements Request +{ + private final HttpServletRequest _servletRequest; + private final ServletContextRequest _servletContextRequest; + private final HttpFields _httpFields; + private final HttpURI _uri; + + ServletCoreRequest(HttpServletRequest request) + { + _servletRequest = request; + _servletContextRequest = ServletContextRequest.getServletContextRequest(_servletRequest); + + HttpFields.Mutable fields = HttpFields.build(); + + Enumeration headerNames = request.getHeaderNames(); + while (headerNames.hasMoreElements()) + { + String headerName = headerNames.nextElement(); + Enumeration headerValues = request.getHeaders(headerName); + while (headerValues.hasMoreElements()) + { + String headerValue = headerValues.nextElement(); + fields.add(new HttpField(headerName, headerValue)); + } + } + + _httpFields = fields.asImmutable(); + String includedServletPath = (String)request.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); + boolean included = includedServletPath != null; + + HttpURI.Mutable builder = HttpURI.build(request.getRequestURI()); + if (included) + builder.path(addEncodedPaths(request.getContextPath(), encodePath(DefaultServlet.getIncludedPathInContext(request, includedServletPath, false)))); + else if (request.getDispatcherType() != DispatcherType.REQUEST) + builder.path(addEncodedPaths(request.getContextPath(), encodePath(URIUtil.addPaths(_servletRequest.getServletPath(), _servletRequest.getPathInfo())))); + builder.query(request.getQueryString()); + _uri = builder.asImmutable(); + } + + @Override + public HttpFields getHeaders() + { + return _httpFields; + } + + @Override + public HttpURI getHttpURI() + { + return _uri; + } + + @Override + public String getId() + { + return _servletRequest.getRequestId(); + } + + @Override + public String getMethod() + { + return _servletRequest.getMethod(); + } + + public HttpServletRequest getServletRequest() + { + return _servletRequest; + } + + @Override + public boolean isSecure() + { + return _servletRequest.isSecure(); + } + + @Override + public Object removeAttribute(String name) + { + Object value = _servletRequest.getAttribute(name); + _servletRequest.removeAttribute(name); + return value; + } + + @Override + public Object setAttribute(String name, Object attribute) + { + Object value = _servletRequest.getAttribute(name); + _servletRequest.setAttribute(name, attribute); + return value; + } + + @Override + public Object getAttribute(String name) + { + return _servletRequest.getAttribute(name); + } + + @Override + public Set getAttributeNameSet() + { + Set set = new HashSet<>(); + Enumeration e = _servletRequest.getAttributeNames(); + while (e.hasMoreElements()) + { + set.add(e.nextElement()); + } + return set; + } + + @Override + public void clearAttributes() + { + Enumeration e = _servletRequest.getAttributeNames(); + while (e.hasMoreElements()) + { + _servletRequest.removeAttribute(e.nextElement()); + } + } + + @Override + public void fail(Throwable failure) + { + throw new UnsupportedOperationException(); + } + + @Override + public Components getComponents() + { + return _servletContextRequest.getComponents(); + } + + @Override + public ConnectionMetaData getConnectionMetaData() + { + return _servletContextRequest.getConnectionMetaData(); + } + + @Override + public Context getContext() + { + return _servletContextRequest.getContext(); + } + + @Override + public void demand(Runnable demandCallback) + { + throw new UnsupportedOperationException(); + } + + @Override + public HttpFields getTrailers() + { + return _servletContextRequest.getTrailers(); + } + + @Override + public long getBeginNanoTime() + { + return _servletContextRequest.getBeginNanoTime(); + } + + @Override + public long getHeadersNanoTime() + { + return _servletContextRequest.getHeadersNanoTime(); + } + + @Override + public Content.Chunk read() + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean consumeAvailable() + { + throw new UnsupportedOperationException(); + } + + @Override + public void addIdleTimeoutListener(Predicate onIdleTimeout) + { + _servletContextRequest.addIdleTimeoutListener(onIdleTimeout); + } + + @Override + public void addFailureListener(Consumer onFailure) + { + _servletContextRequest.addFailureListener(onFailure); + } + + @Override + public TunnelSupport getTunnelSupport() + { + return null; + } + + @Override + public void addHttpStreamWrapper(Function wrapper) + { + _servletContextRequest.addHttpStreamWrapper(wrapper); + } + + @Override + public Session getSession(boolean create) + { + return Session.getSession(_servletRequest.getSession(create)); + } +} diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreResponse.java new file mode 100644 index 00000000000..a11ae77b6bb --- /dev/null +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreResponse.java @@ -0,0 +1,379 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.ee10.servlet; + +import java.io.InputStreamReader; +import java.nio.ByteBuffer; +import java.util.EnumSet; +import java.util.List; +import java.util.ListIterator; +import java.util.Objects; +import java.util.concurrent.CompletableFuture; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpHeaderValue; +import org.eclipse.jetty.io.ByteBufferInputStream; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.IO; + +/** + * A {@link HttpServletResponse} wrapped as a core {@link Response}. + * All write operations are internally converted to blocking writes on the servlet API. + */ +class ServletCoreResponse implements Response +{ + private final HttpServletResponse _response; + private final Request _coreRequest; + private final HttpFields.Mutable _httpFields; + private final boolean _included; + private final ServletContextResponse _servletContextResponse; + + public ServletCoreResponse(Request coreRequest, HttpServletResponse response, boolean included) + { + _coreRequest = coreRequest; + _response = response; + _servletContextResponse = ServletContextResponse.getServletContextResponse(response); + HttpFields.Mutable fields = new HttpServletResponseHttpFields(response); + if (included) + { + // If included, accept but ignore mutations. + fields = new HttpFields.Mutable.Wrapper(fields) + { + @Override + public HttpField onAddField(HttpField field) + { + return null; + } + + @Override + public boolean onRemoveField(HttpField field) + { + return false; + } + }; + } + _httpFields = fields; + _included = included; + } + + @Override + public HttpFields.Mutable getHeaders() + { + return _httpFields; + } + + public HttpServletResponse getServletResponse() + { + return _response; + } + + @Override + public boolean hasLastWrite() + { + return _servletContextResponse.hasLastWrite(); + } + + @Override + public boolean isCompletedSuccessfully() + { + return _servletContextResponse.isCompletedSuccessfully(); + } + + @Override + public boolean isCommitted() + { + return _response.isCommitted(); + } + + private boolean isWriting() + { + return _servletContextResponse.isWriting(); + } + + @Override + public void write(boolean last, ByteBuffer byteBuffer, Callback callback) + { + if (_included) + last = false; + try + { + if (BufferUtil.hasContent(byteBuffer)) + { + if (isWriting()) + { + String characterEncoding = _response.getCharacterEncoding(); + try (ByteBufferInputStream bbis = new ByteBufferInputStream(byteBuffer); + InputStreamReader reader = new InputStreamReader(bbis, characterEncoding)) + { + IO.copy(reader, _response.getWriter()); + } + + if (last) + _response.getWriter().close(); + } + else + { + BufferUtil.writeTo(byteBuffer, _response.getOutputStream()); + if (last) + _response.getOutputStream().close(); + } + } + + callback.succeeded(); + } + catch (Throwable t) + { + callback.failed(t); + } + } + + @Override + public Request getRequest() + { + return _coreRequest; + } + + @Override + public int getStatus() + { + return _response.getStatus(); + } + + @Override + public void setStatus(int code) + { + if (_included) + return; + _response.setStatus(code); + } + + @Override + public Supplier getTrailersSupplier() + { + return null; + } + + @Override + public void setTrailersSupplier(Supplier trailers) + { + } + + @Override + public void reset() + { + _response.reset(); + } + + @Override + public CompletableFuture writeInterim(int status, HttpFields headers) + { + throw new UnsupportedOperationException(); + } + + @Override + public String toString() + { + return "%s@%x{%s,%s}".formatted(this.getClass().getSimpleName(), hashCode(), this._coreRequest, _response); + } + + private static class HttpServletResponseHttpFields implements HttpFields.Mutable + { + private final HttpServletResponse _response; + + private HttpServletResponseHttpFields(HttpServletResponse response) + { + _response = response; + } + + @Override + public ListIterator listIterator() + { + // The minimum requirement is to implement the listIterator, but it is inefficient. + // Other methods are implemented for efficiency. + final ListIterator list = _response.getHeaderNames().stream() + .map(n -> new HttpField(n, _response.getHeader(n))) + .collect(Collectors.toList()) + .listIterator(); + + return new ListIterator<>() + { + HttpField _last; + + @Override + public boolean hasNext() + { + return list.hasNext(); + } + + @Override + public HttpField next() + { + return _last = list.next(); + } + + @Override + public boolean hasPrevious() + { + return list.hasPrevious(); + } + + @Override + public HttpField previous() + { + return _last = list.previous(); + } + + @Override + public int nextIndex() + { + return list.nextIndex(); + } + + @Override + public int previousIndex() + { + return list.previousIndex(); + } + + @Override + public void remove() + { + if (_last != null) + { + // This is not exactly the right semantic for repeated field names + list.remove(); + _response.setHeader(_last.getName(), null); + } + } + + @Override + public void set(HttpField httpField) + { + list.set(httpField); + _response.setHeader(httpField.getName(), httpField.getValue()); + } + + @Override + public void add(HttpField httpField) + { + list.add(httpField); + _response.addHeader(httpField.getName(), httpField.getValue()); + } + }; + } + + @Override + public Mutable add(String name, String value) + { + _response.addHeader(name, value); + return this; + } + + @Override + public Mutable add(HttpHeader header, HttpHeaderValue value) + { + _response.addHeader(header.asString(), value.asString()); + return this; + } + + @Override + public Mutable add(HttpHeader header, String value) + { + _response.addHeader(header.asString(), value); + return this; + } + + @Override + public Mutable add(HttpField field) + { + _response.addHeader(field.getName(), field.getValue()); + return this; + } + + @Override + public Mutable put(HttpField field) + { + _response.setHeader(field.getName(), field.getValue()); + return this; + } + + @Override + public Mutable put(String name, String value) + { + _response.setHeader(name, value); + return this; + } + + @Override + public Mutable put(HttpHeader header, HttpHeaderValue value) + { + _response.setHeader(header.asString(), value.asString()); + return this; + } + + @Override + public Mutable put(HttpHeader header, String value) + { + _response.setHeader(header.asString(), value); + return this; + } + + @Override + public Mutable put(String name, List list) + { + Objects.requireNonNull(name); + Objects.requireNonNull(list); + boolean first = true; + for (String s : list) + { + if (first) + _response.setHeader(name, s); + else + _response.addHeader(name, s); + first = false; + } + return this; + } + + @Override + public Mutable remove(HttpHeader header) + { + _response.setHeader(header.asString(), null); + return this; + } + + @Override + public Mutable remove(EnumSet fields) + { + for (HttpHeader header : fields) + remove(header); + return this; + } + + @Override + public Mutable remove(String name) + { + _response.setHeader(name, null); + return this; + } + } +} diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java index 05c2b6476a9..38c8586dbd4 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java @@ -2035,7 +2035,7 @@ public class DefaultServletTest String body = response.getContent(); assertThat(response, containsHeaderValue("Content-Type", "multipart/byteranges")); - assertThat(response, containsHeaderValue("Content-Length", "" + body.length())); + // TODO #10307 assertThat(response, containsHeaderValue("Content-Length", String.valueOf(body.length()))); HttpField contentType = response.getField(HttpHeader.CONTENT_TYPE); String boundary = getContentTypeBoundary(contentType); @@ -2063,7 +2063,7 @@ public class DefaultServletTest String body = response.getContent(); assertThat(response, containsHeaderValue("Content-Type", "multipart/byteranges")); - assertThat(response, containsHeaderValue("Content-Length", "" + body.length())); + // TODO #10307 assertThat(response, containsHeaderValue("Content-Length", String.valueOf(body.length()))); HttpField contentType = response.getField(HttpHeader.CONTENT_TYPE); String boundary = getContentTypeBoundary(contentType); @@ -2093,7 +2093,7 @@ public class DefaultServletTest String body = response.getContent(); assertThat(response, containsHeaderValue("Content-Type", "multipart/byteranges")); - assertThat(response, containsHeaderValue("Content-Length", "" + body.length())); + // TODO #10307 assertThat(response, containsHeaderValue("Content-Length", String.valueOf(body.length()))); HttpField contentType = response.getField(HttpHeader.CONTENT_TYPE); String boundary = getContentTypeBoundary(contentType); @@ -2154,7 +2154,7 @@ public class DefaultServletTest String body = response.getContent(); assertThat(response, containsHeaderValue("Content-Type", "multipart/byteranges")); - assertThat(response, containsHeaderValue("Content-Length", "" + body.length())); + // TODO #10307 assertThat(response, containsHeaderValue("Content-Length", String.valueOf(body.length()))); HttpField contentType = response.getField(HttpHeader.CONTENT_TYPE); String boundary = getContentTypeBoundary(contentType); @@ -2183,7 +2183,7 @@ public class DefaultServletTest String body = response.getContent(); assertThat(response, containsHeaderValue("Content-Type", "multipart/byteranges")); - assertThat(response, containsHeaderValue("Content-Length", "" + body.length())); + // TODO #10307 assertThat(response, containsHeaderValue("Content-Length", String.valueOf(body.length()))); HttpField contentType = response.getField(HttpHeader.CONTENT_TYPE); String boundary = getContentTypeBoundary(contentType); @@ -2295,7 +2295,7 @@ public class DefaultServletTest HttpTester.Response response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); String body = response.getContent(); - assertThat(response, containsHeaderValue(HttpHeader.CONTENT_LENGTH, "" + body.length())); + assertThat(response, containsHeaderValue(HttpHeader.CONTENT_LENGTH, String.valueOf(body.length()))); assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "text/plain;charset=UTF-8")); assertThat(body, containsString("Extra Info")); @@ -2308,7 +2308,7 @@ public class DefaultServletTest response = HttpTester.parseResponse(rawResponse); assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); body = response.getContent(); - assertThat(response, containsHeaderValue(HttpHeader.CONTENT_LENGTH, "" + body.length())); + assertThat(response, containsHeaderValue(HttpHeader.CONTENT_LENGTH, String.valueOf(body.length()))); assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "image/jpeg;charset=utf-8")); assertThat(body, containsString("Extra Info")); } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpOutput.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpOutput.java index 5113c845ff2..62744880a71 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpOutput.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpOutput.java @@ -1585,8 +1585,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable } catch (Throwable t) { - if (ExceptionUtil.areNotAssociated(e, t)) - e.addSuppressed(t); + ExceptionUtil.addSuppressedIfNotAssociated(e, t); } finally { diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java index 989dbf595a8..ff98449092c 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java @@ -2482,6 +2482,12 @@ public class ResponseTest return _committed; } + @Override + public boolean hasLastWrite() + { + return _last; + } + @Override public boolean isCompletedSuccessfully() { From e33d02625900b617ac3cf9187002faa80a776db3 Mon Sep 17 00:00:00 2001 From: yokotaso Date: Wed, 16 Aug 2023 09:06:35 +0900 Subject: [PATCH 24/31] chore: Fix javadoc following the source code --- .../src/main/java/org/eclipse/jetty/http/UriCompliance.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java index e2fde79d828..bb75d8bff42 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java @@ -102,7 +102,8 @@ public final class UriCompliance implements ComplianceViolation.Mode /** * The default compliance mode that extends RFC3986 compliance with * additional violations to avoid most ambiguous URIs. - * This mode does allow {@link Violation#AMBIGUOUS_PATH_SEPARATOR}, but disallows all out {@link Violation}s. + * This mode does allow {@link Violation#AMBIGUOUS_PATH_SEPARATOR}, {@link Violation#AMBIGUOUS_PATH_ENCODING}, + * but disallows all out {@link Violation}s. */ public static final UriCompliance DEFAULT = new UriCompliance("DEFAULT", of(Violation.AMBIGUOUS_PATH_SEPARATOR, From cab3128d530b28bbe2679da5bda524d0d19541a2 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 17 Aug 2023 17:15:52 +0200 Subject: [PATCH 25/31] Fixed POM dependency. Signed-off-by: Simone Bordet --- .../jetty-ee9-websocket-jetty-tests/pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/pom.xml b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/pom.xml index 49bbb350455..e8e5348e0df 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/pom.xml +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/pom.xml @@ -73,8 +73,8 @@ test - org.eclipse.jetty - jetty-annotations + org.eclipse.jetty.ee9 + jetty-ee9-annotations test From 7cd55498e2bf211ce97b22b569a1f008a51e39c7 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 17 Aug 2023 16:48:59 -0500 Subject: [PATCH 26/31] Issue #10158 - tests to prove out XML only based deployment technique (#10209) --- .../tests/distribution/DistributionTests.java | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java b/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java index ea06afc67e5..71f1ca03304 100644 --- a/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java +++ b/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java @@ -50,6 +50,7 @@ import org.eclipse.jetty.io.ClientConnector; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.content.ByteBufferContentSource; import org.eclipse.jetty.tests.hometester.JettyHomeTester; +import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.PathMatchers; import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.util.ssl.SslContextFactory; @@ -1529,4 +1530,85 @@ public class DistributionTests extends AbstractJettyHomeTest } } } + + @ParameterizedTest + @ValueSource(strings = {"ee8", "ee9", "ee10"}) + public void testXmlDeployWarNotInWebapps(String env) throws Exception + { + Path jettyBase = newTestJettyBaseDirectory(); + String jettyVersion = System.getProperty("jettyVersion"); + JettyHomeTester distribution = JettyHomeTester.Builder.newInstance() + .jettyVersion(jettyVersion) + .jettyBase(jettyBase) + .mavenLocalRepository(System.getProperty("mavenRepoPath")) + .build(); + + int httpPort = distribution.freePort(); + + String[] argsConfig = { + "--add-modules=http," + toEnvironment("deploy", env) + "," + toEnvironment("webapp", env) + }; + + try (JettyHomeTester.Run runConfig = distribution.start(argsConfig)) + { + assertTrue(runConfig.awaitFor(START_TIMEOUT, TimeUnit.SECONDS)); + assertEquals(0, runConfig.getExitValue()); + + String[] argsStart = { + "jetty.http.port=" + httpPort, + "jetty.httpConfig.port=" + httpPort + }; + + // Put war into ${jetty.base}/wars/ directory + File srcWar = distribution.resolveArtifact("org.eclipse.jetty." + env + ".demos:jetty-" + env + "-demo-simple-webapp:war:" + jettyVersion); + Path warsDir = jettyBase.resolve("wars"); + FS.ensureDirExists(warsDir); + Path destWar = warsDir.resolve("demo.war"); + Files.copy(srcWar.toPath(), destWar); + + // Create XML for deployable + String xml = """ + + + + + /demo + %s + + """.formatted(env, destWar.toString()); + Files.writeString(jettyBase.resolve("webapps/demo.xml"), xml, StandardCharsets.UTF_8); + + // Specify Environment Properties for this raw XML based deployable + String props = """ + environment=%s + """.formatted(env); + Files.writeString(jettyBase.resolve("webapps/demo.properties"), props, StandardCharsets.UTF_8); + + /* The jetty.base tree should now look like this + * + * ${jetty.base} + * ├── resources/ + * │ └── jetty-logging.properties + * ├── start.d/ + * │ ├── ${env}-deploy.ini + * │ ├── ${env}-webapp.ini + * │ └── http.ini + * ├── wars/ + * │ └── demo.war + * ├── webapps/ + * │ ├── demo.properties + * │ └── demo.xml + * └── work/ + */ + + try (JettyHomeTester.Run runStart = distribution.start(argsStart)) + { + assertTrue(runStart.awaitConsoleLogsFor("Started oejs.Server@", START_TIMEOUT, TimeUnit.SECONDS)); + + startHttpClient(); + ContentResponse response = client.GET("http://localhost:" + httpPort + "/demo/index.html"); + assertEquals(HttpStatus.OK_200, response.getStatus()); + } + } + } } From 042841a7c6bd1ee847ee5cdbab8c49d5f27973aa Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 18 Aug 2023 10:11:39 +1000 Subject: [PATCH 27/31] Fix #10306 getServerHost (#10311) Fix #10306 getServerHost --- .../server/internal/ServerFCGIConnection.java | 7 -- .../internal/HTTP2ServerConnection.java | 7 -- .../jetty/server/ConnectionMetaData.java | 17 ++-- .../org/eclipse/jetty/server/Request.java | 40 +++++---- .../jetty/server/internal/HttpConnection.java | 9 -- .../ForwardedRequestCustomizerTest.java | 84 +++++++++++++------ .../jetty/ee10/test/rfcs/RFC2616BaseTest.java | 8 +- .../jetty/ee9/test/rfcs/RFC2616BaseTest.java | 8 +- 8 files changed, 101 insertions(+), 79 deletions(-) diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java index 8f517c09014..25b85a60fca 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java @@ -36,7 +36,6 @@ import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.util.Attributes; -import org.eclipse.jetty.util.HostPort; import org.eclipse.jetty.util.StringUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -156,12 +155,6 @@ public class ServerFCGIConnection extends AbstractConnection implements Connecti return getEndPoint().getLocalSocketAddress(); } - @Override - public HostPort getServerAuthority() - { - return ConnectionMetaData.getServerAuthority(configuration, this); - } - @Override public Object removeAttribute(String name) { diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java index 289bc28a503..3ea92b23caa 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java +++ b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java @@ -51,7 +51,6 @@ import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; -import org.eclipse.jetty.util.HostPort; import org.eclipse.jetty.util.Promise; import org.eclipse.jetty.util.StringUtil; import org.slf4j.Logger; @@ -417,12 +416,6 @@ public class HTTP2ServerConnection extends HTTP2Connection implements Connection return getEndPoint().getLocalSocketAddress(); } - @Override - public HostPort getServerAuthority() - { - return ConnectionMetaData.getServerAuthority(httpConfig, this); - } - @Override public Object getAttribute(String name) { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ConnectionMetaData.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ConnectionMetaData.java index 7c7b5132050..c48a010082d 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ConnectionMetaData.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ConnectionMetaData.java @@ -67,20 +67,21 @@ public interface ConnectionMetaData extends Attributes /** * @return The URI authority that this server represents. By default, this is the address of the network socket on - * which the connection was accepted, but it may be wrapped to represent a virtual address. + * which the connection was accepted, but it may be configured to a specific address. + * @see HttpConfiguration#setServerAuthority(HostPort) */ - HostPort getServerAuthority(); - - static HostPort getServerAuthority(HttpConfiguration httpConfiguration, ConnectionMetaData connectionMetaData) + default HostPort getServerAuthority() { + HttpConfiguration httpConfiguration = getHttpConfiguration(); HostPort authority = httpConfiguration.getServerAuthority(); if (authority != null) return authority; - SocketAddress local = connectionMetaData.getLocalSocketAddress(); - if (local instanceof InetSocketAddress inet) - return new HostPort(inet.getHostString(), inet.getPort()); - + SocketAddress localSocketAddress = getLocalSocketAddress(); + if (localSocketAddress instanceof InetSocketAddress inetSocketAddress) + return new HostPort(inetSocketAddress.getHostString(), inetSocketAddress.getPort()); + else if (localSocketAddress != null) + return new HostPort(localSocketAddress.toString()); return null; } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index c9ff22c9597..6d6d2ccbe6f 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -403,6 +403,12 @@ public interface Request extends Attributes, Content.Source return -1; } + /** + * Get the logical name the request was sent to, which may be from the authority of the + * request; the configured server authority; the actual network name of the server; + * @param request The request to get the server name of + * @return The logical server name or null if it cannot be determined. + */ static String getServerName(Request request) { if (request == null) @@ -414,38 +420,42 @@ public interface Request extends Attributes, Content.Source HostPort authority = request.getConnectionMetaData().getServerAuthority(); if (authority != null) - return HostPort.normalizeHost(authority.getHost()); + return authority.getHost(); - SocketAddress local = request.getConnectionMetaData().getLocalSocketAddress(); - if (local instanceof InetSocketAddress) - return HostPort.normalizeHost(((InetSocketAddress)local).getHostString()); - - return local == null ? null : local.toString(); + return null; } + /** + * Get the logical port a request was received on, which may be from the authority of the request; the + * configured server authority; the default port for the scheme; or the actual network port. + * @param request The request to get the port of + * @return The port for the request if it can be determined, otherwise -1 + */ static int getServerPort(Request request) { if (request == null) return -1; + + // Does the request have an explicit port? HttpURI uri = request.getHttpURI(); if (uri.hasAuthority() && uri.getPort() > 0) return uri.getPort(); - HostPort authority = request.getConnectionMetaData().getServerAuthority(); + // Is there a configured server authority? + HostPort authority = request.getConnectionMetaData().getHttpConfiguration().getServerAuthority(); if (authority != null && authority.getPort() > 0) return authority.getPort(); - if (authority == null) - { - SocketAddress local = request.getConnectionMetaData().getLocalSocketAddress(); - if (local instanceof InetSocketAddress) - return ((InetSocketAddress)local).getPort(); - } - + // Is there a scheme with a default port? HttpScheme scheme = HttpScheme.CACHE.get(request.getHttpURI().getScheme()); - if (scheme != null) + if (scheme != null && scheme.getDefaultPort() > 0) return scheme.getDefaultPort(); + // Is there a local port? + SocketAddress local = request.getConnectionMetaData().getLocalSocketAddress(); + if (local instanceof InetSocketAddress inetSocketAddress && inetSocketAddress.getPort() > 0) + return inetSocketAddress.getPort(); + return -1; } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java index 3034d4d1f9f..8c738937568 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java @@ -288,15 +288,6 @@ public class HttpConnection extends AbstractConnection implements Runnable, Writ return getEndPoint().getLocalSocketAddress(); } - @Override - public HostPort getServerAuthority() - { - HostPort authority = ConnectionMetaData.getServerAuthority(getHttpConfiguration(), this); - if (authority == null) - authority = new HostPort(getLocalSocketAddress().toString(), -1); - return authority; - } - @Override public Object removeAttribute(String name) { diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index bd721995668..7f1e65270f7 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -14,6 +14,8 @@ package org.eclipse.jetty.server; import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.SocketAddress; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -22,6 +24,9 @@ import java.util.stream.Stream; import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.io.Connection; +import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.server.internal.HttpConnection; import org.eclipse.jetty.util.Callback; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -75,7 +80,25 @@ public class ForwardedRequestCustomizerTest server.addConnector(connector); // Alternate behavior Connector - HttpConnectionFactory httpAlt = new HttpConnectionFactory(); + HttpConnectionFactory httpAlt = new HttpConnectionFactory() + { + @Override + public Connection newConnection(Connector connector, EndPoint endPoint) + { + HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint, isRecordHttpComplianceViolations()) + { + @Override + public SocketAddress getLocalSocketAddress() + { + return InetSocketAddress.createUnresolved("test", 42); + } + }; + connection.setUseInputDirectByteBuffers(isUseInputDirectByteBuffers()); + connection.setUseOutputDirectByteBuffers(isUseOutputDirectByteBuffers()); + return configure(connection, connector, endPoint); + } + }; + httpAlt.getHttpConfiguration().setSecurePort(8443); httpAlt.getHttpConfiguration().setHttpCompliance(mismatchedAuthorityHttpCompliance); customizerAlt = new ForwardedRequestCustomizer(); @@ -128,27 +151,6 @@ public class ForwardedRequestCustomizerTest server.stop(); } - public static Stream cases2() - { - return Stream.of( - Arguments.of(new TestRequest("https initial authority, X-Forwarded-Proto on http, Proxy-Ssl-Id exists (setSslIsSecure==false)") - .configureCustomizer((customizer) -> customizer.setSslIsSecure(false)) - .headers( - "GET https://alt.example.net/foo HTTP/1.1", - "Host: alt.example.net", - "X-Forwarded-Proto: http", - "Proxy-Ssl-Id: Wibble" - ), - new Expectations() - .scheme("http").serverName("alt.example.net").serverPort(80) - .secure(false) - .requestURL("http://alt.example.net/foo") - .remoteAddr("0.0.0.0").remotePort(0) - .sslSession("Wibble") - ) - ); - } - public static Stream cases() { return Stream.of( @@ -1032,7 +1034,6 @@ public class ForwardedRequestCustomizerTest request.configure(customizer); String rawRequest = request.getRawRequest((header) -> header); -// System.out.println(rawRequest); HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(rawRequest)); assertThat("status", response.getStatus(), is(200)); @@ -1052,7 +1053,6 @@ public class ForwardedRequestCustomizerTest .replaceFirst("X-Proxied-Https:", "Jetty-Proxied-Https:") .replaceFirst("Proxy-Ssl-Id:", "Jetty-Proxy-Ssl-Id:") .replaceFirst("Proxy-auth-cert:", "Jetty-Proxy-Auth-Cert:")); - // System.out.println(rawRequest); HttpTester.Response response = HttpTester.parseResponse(connectorConfigured.getResponse(rawRequest)); assertThat("status", response.getStatus(), is(200)); @@ -1063,6 +1063,40 @@ public class ForwardedRequestCustomizerTest public static Stream nonStandardPortCases() { return Stream.of( + // HTTP 1.0 + Arguments.of( + new TestRequest("HTTP/1.0 - no Host header") + .headers( + "GET /example HTTP/1.0" + ), + new Expectations() + .scheme("http").serverName("test").serverPort(42) + .secure(false) + .requestURL("http://test:42/example") + ), + Arguments.of( + new TestRequest("HTTP/1.0 - Empty Host header") + .headers( + "GET scheme:///example HTTP/1.0", + "Host:" + ), + new Expectations() + .scheme("scheme").serverName(null).serverPort(42) + .secure(false) + .requestURL("scheme:///example") + ), + Arguments.of( + new TestRequest("HTTP/1.0 - Host header") + .headers( + "GET /example HTTP/1.0", + "Host: server:9999" + ), + new Expectations() + .scheme("http").serverName("server").serverPort(9999) + .secure(false) + .requestURL("http://server:9999/example") + ), + // RFC7239 Tests with https. Arguments.of(new TestRequest("RFC7239 with https and h2") .headers( @@ -1106,7 +1140,7 @@ public class ForwardedRequestCustomizerTest /** * Tests against a Connector with a HttpConfiguration on non-standard ports. - * HttpConfiguration is set to securePort of 8443 + * HttpConfiguration is set to securePort of 8443 and the local port is 42. */ @ParameterizedTest(name = "{0}") @MethodSource("nonStandardPortCases") diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/rfcs/RFC2616BaseTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/rfcs/RFC2616BaseTest.java index 710ab5bd80b..b05bd09d089 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/rfcs/RFC2616BaseTest.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/rfcs/RFC2616BaseTest.java @@ -1117,12 +1117,12 @@ public abstract class RFC2616BaseTest HttpTester.Response response = responses.get(0); String specId = "10.3 Redirection HTTP/1.1 - basic (response 1)"; assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302)); - assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/", response.get("Location"), specId); + assertEquals(getServer().getScheme() + "://localhost/tests/", response.get("Location"), specId); response = responses.get(1); specId = "10.3 Redirection HTTP/1.1 - basic (response 2)"; assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302)); - assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/", response.get("Location"), specId); + assertEquals(getServer().getScheme() + "://localhost/tests/", response.get("Location"), specId); assertEquals("close", response.get("Connection"), specId); } @@ -1146,7 +1146,7 @@ public abstract class RFC2616BaseTest String specId = "10.3 Redirection HTTP/1.0 w/content"; assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302)); - assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/R1.txt", response.get("Location"), specId); + assertEquals(getServer().getScheme() + "://localhost/tests/R1.txt", response.get("Location"), specId); } /** @@ -1169,7 +1169,7 @@ public abstract class RFC2616BaseTest String specId = "10.3 Redirection HTTP/1.1 w/content"; assertThat(specId + " [status]", response.getStatus(), is(HttpStatus.FOUND_302)); - assertThat(specId + " [location]", response.get("Location"), is(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/R2.txt")); + assertThat(specId + " [location]", response.get("Location"), is(getServer().getScheme() + "://localhost/tests/R2.txt")); assertThat(specId + " [connection]", response.get("Connection"), is("close")); } diff --git a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/rfcs/RFC2616BaseTest.java b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/rfcs/RFC2616BaseTest.java index 4f0bf90480d..25b633191c5 100644 --- a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/rfcs/RFC2616BaseTest.java +++ b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/rfcs/RFC2616BaseTest.java @@ -1114,12 +1114,12 @@ public abstract class RFC2616BaseTest HttpTester.Response response = responses.get(0); String specId = "10.3 Redirection HTTP/1.1 - basic (response 1)"; assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302)); - assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/", response.get("Location"), specId); + assertEquals(getServer().getScheme() + "://localhost/tests/", response.get("Location"), specId); response = responses.get(1); specId = "10.3 Redirection HTTP/1.1 - basic (response 2)"; assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302)); - assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/", response.get("Location"), specId); + assertEquals(getServer().getScheme() + "://localhost/tests/", response.get("Location"), specId); assertEquals("close", response.get("Connection"), specId); } @@ -1143,7 +1143,7 @@ public abstract class RFC2616BaseTest String specId = "10.3 Redirection HTTP/1.0 w/content"; assertThat(specId, response.getStatus(), is(HttpStatus.FOUND_302)); - assertEquals(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/R1.txt", response.get("Location"), specId); + assertEquals(getServer().getScheme() + "://localhost/tests/R1.txt", response.get("Location"), specId); } /** @@ -1166,7 +1166,7 @@ public abstract class RFC2616BaseTest String specId = "10.3 Redirection HTTP/1.1 w/content"; assertThat(specId + " [status]", response.getStatus(), is(HttpStatus.FOUND_302)); - assertThat(specId + " [location]", response.get("Location"), is(getServer().getScheme() + "://localhost:" + getServer().getServerPort() + "/tests/R2.txt")); + assertThat(specId + " [location]", response.get("Location"), is(getServer().getScheme() + "://localhost/tests/R2.txt")); assertThat(specId + " [connection]", response.get("Connection"), is("close")); } From 33ef0b040a39d21657ad30d5b92b2aa0c847d7c2 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 18 Aug 2023 03:18:29 +0200 Subject: [PATCH 28/31] Issue #10158 Change ignored deployment message to WARN (#10321) --- .../eclipse/jetty/deploy/providers/ScanningAppProvider.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/jetty-core/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java b/jetty-core/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java index 0d6bc46b254..0c3757e55a2 100644 --- a/jetty-core/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java +++ b/jetty-core/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java @@ -197,8 +197,7 @@ public abstract class ScanningAppProvider extends ContainerLifeCycle implements } } - if (LOG.isDebugEnabled()) - LOG.debug("{} ignored {}", this, app); + LOG.warn("{} no environment for {}, ignoring", this, app); return null; } From 5160ad29ef69f1302abb536067e2f4575bba01e5 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 18 Aug 2023 03:19:06 +0200 Subject: [PATCH 29/31] Issue #10207 Update missing JSP message. (#10320) * Issue #10207 Update missing JSP message. --- .../main/java/org/eclipse/jetty/ee10/servlet/NoJspServlet.java | 2 +- .../main/java/org/eclipse/jetty/ee9/servlet/NoJspServlet.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/NoJspServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/NoJspServlet.java index 05dda4f5992..c23dea5d190 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/NoJspServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/NoJspServlet.java @@ -28,7 +28,7 @@ public class NoJspServlet extends HttpServlet protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException { if (!_warned) - getServletContext().log("No JSP support. Check that JSP jars are in lib/jsp and that the JSP option has been specified to start.jar"); + getServletContext().log("No JSP support. Check that the ee10-jsp module is enabled, or otherwise ensure the jsp jars are on the server classpath."); _warned = true; response.sendError(500, "JSP support not configured"); diff --git a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/NoJspServlet.java b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/NoJspServlet.java index ea0e98e3b59..cde119d8e11 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/NoJspServlet.java +++ b/jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/NoJspServlet.java @@ -28,7 +28,7 @@ public class NoJspServlet extends HttpServlet protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException { if (!_warned) - getServletContext().log("No JSP support. Check that JSP jars are in lib/jsp and that the JSP option has been specified to start.jar"); + getServletContext().log("No JSP support. Check that the ee9-jsp module is enabled, or otherwise ensure the jsp jars are on the server classpath."); _warned = true; response.sendError(500, "JSP support not configured"); From fe1831008a8c1a1a5af6d876c0380e1b126476e1 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 18 Aug 2023 03:19:53 +0200 Subject: [PATCH 30/31] Issue #10323 Fix ee10 Request.isRequestedSessionIdValid (#10331) * Issue #10323 Fix ee10 Request.isRequestedSessionIdValid --- .../jetty/ee10/servlet/ServletApiRequest.java | 2 +- .../ee10/servlet/SessionHandlerTest.java | 194 +++++++++++++++++- .../jetty/ee9/nested/SessionHandlerTest.java | 76 ++++++- 3 files changed, 258 insertions(+), 14 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java index 41702d09266..59c85855758 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java @@ -412,7 +412,7 @@ public class ServletApiRequest implements HttpServletRequest public boolean isRequestedSessionIdValid() { AbstractSessionManager.RequestedSession requestedSession = getServletRequestInfo().getRequestedSession(); - return requestedSession != null && requestedSession.sessionId() != null && !requestedSession.sessionIdFromCookie(); + return requestedSession != null && requestedSession.sessionId() != null && requestedSession.session() != null; } @Override diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SessionHandlerTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SessionHandlerTest.java index e3f1042f53d..133845af7b2 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SessionHandlerTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SessionHandlerTest.java @@ -16,6 +16,8 @@ package org.eclipse.jetty.ee10.servlet; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.io.PrintWriter; +import java.net.URI; import java.net.URL; import java.net.URLClassLoader; import java.nio.file.Files; @@ -23,6 +25,7 @@ import java.nio.file.Path; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.Map; import java.util.function.Consumer; import java.util.function.Function; @@ -57,6 +60,7 @@ import org.eclipse.jetty.session.SessionDataStoreFactory; import org.eclipse.jetty.toolchain.test.IO; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; +import org.eclipse.jetty.util.URIUtil; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -359,6 +363,162 @@ public class SessionHandlerTest } } + @Test + public void testRequestedSessionIdFromCookie() throws Exception + { + String contextPath = "/"; + String servletMapping = "/server"; + + Server server = new Server(); + ServerConnector connector = new ServerConnector(server); + server.addConnector(connector); + + DefaultSessionIdManager sessionIdManager = new DefaultSessionIdManager(server); + server.addBean(sessionIdManager, true); + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); + server.addBean(cacheFactory); + + SessionDataStoreFactory storeFactory = new NullSessionDataStoreFactory(); + server.addBean(storeFactory); + + HouseKeeper housekeeper = new HouseKeeper(); + housekeeper.setIntervalSec(-1); //turn off scavenging + sessionIdManager.setSessionHouseKeeper(housekeeper); + + ServletContextHandler context = new ServletContextHandler(); + context.setContextPath(contextPath); + server.setHandler(context); + SessionHandler sessionHandler = new SessionHandler(); + sessionHandler.setSessionIdManager(sessionIdManager); + sessionHandler.setMaxInactiveInterval(-1); //immortal session + context.setSessionHandler(sessionHandler); + + TestRequestedSessionIdServlet servlet = new TestRequestedSessionIdServlet(); + ServletHolder holder = new ServletHolder(servlet); + context.addServlet(holder, servletMapping); + + server.start(); + int port = connector.getLocalPort(); + try (StacklessLogging stackless = new StacklessLogging(SessionHandlerTest.class.getPackage())) + { + HttpClient client = new HttpClient(); + client.start(); + + //test with no session cookie + String path = contextPath + (contextPath.endsWith("/") && servletMapping.startsWith("/") ? servletMapping.substring(1) : servletMapping); + String url = "http://localhost:" + port + path; + ContentResponse response = client.GET(url); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + assertThat(response.getContentAsString(), containsString("valid=false")); + + //test with a cookie for non-existant session + URI uri = URIUtil.toURI(URIUtil.newURI("http", "localhost", port, path, "")); + HttpCookie cookie = HttpCookie.build(SessionHandler.__DefaultSessionCookie, "123456789").path("/").domain("localhost").build(); + client.getHttpCookieStore().add(uri, cookie); + response = client.GET(url); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + String content = response.getContentAsString(); + assertThat(content, containsString("requestedId=123456789")); + assertThat(content, containsString("valid=false")); + + //Get rid of fake cookie + client.getHttpCookieStore().clear(); + + //Make a real session + response = client.GET(url + "?action=create"); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + assertNotNull(response.getHeaders().get("Set-Cookie")); + + //Check the requestedSessionId is valid + response = client.GET(url); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + content = response.getContentAsString(); + assertThat(content, containsString("valid=true")); + } + finally + { + server.stop(); + } + } + + @Test + public void testRequestedSessionIdFromURL() throws Exception + { + String contextPath = "/"; + String servletMapping = "/server"; + + Server server = new Server(); + ServerConnector connector = new ServerConnector(server); + server.addConnector(connector); + + DefaultSessionIdManager sessionIdManager = new DefaultSessionIdManager(server); + server.addBean(sessionIdManager, true); + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); + server.addBean(cacheFactory); + + SessionDataStoreFactory storeFactory = new NullSessionDataStoreFactory(); + server.addBean(storeFactory); + + HouseKeeper housekeeper = new HouseKeeper(); + housekeeper.setIntervalSec(-1); //turn off scavenging + sessionIdManager.setSessionHouseKeeper(housekeeper); + + ServletContextHandler context = new ServletContextHandler(); + context.setContextPath(contextPath); + server.setHandler(context); + SessionHandler sessionHandler = new SessionHandler(); + sessionHandler.setUsingCookies(false); + sessionHandler.setSessionIdManager(sessionIdManager); + sessionHandler.setMaxInactiveInterval(-1); //immortal session + context.setSessionHandler(sessionHandler); + + TestRequestedSessionIdServlet servlet = new TestRequestedSessionIdServlet(); + ServletHolder holder = new ServletHolder(servlet); + context.addServlet(holder, servletMapping); + + server.start(); + int port = connector.getLocalPort(); + try (StacklessLogging stackless = new StacklessLogging(SessionHandlerTest.class.getPackage())) + { + HttpClient client = new HttpClient(); + client.start(); + + //test with no session cookie + String path = contextPath + (contextPath.endsWith("/") && servletMapping.startsWith("/") ? servletMapping.substring(1) : servletMapping); + String url = "http://localhost:" + port + path; + ContentResponse response = client.GET(url); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + assertThat(response.getContentAsString(), containsString("valid=false")); + + //test with id for non-existent session + response = client.GET(url + ";" + SessionHandler.__DefaultSessionIdPathParameterName + "=" + "123456789"); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + String content = response.getContentAsString(); + assertThat(content, containsString("requestedId=123456789")); + assertThat(content, containsString("valid=false")); + + //Make a real session + response = client.GET(url + "?action=create"); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + content = response.getContentAsString(); + assertThat(content, containsString("createdId=")); + String sessionId = content.substring(content.indexOf("createdId=") + 10); + sessionId = sessionId.trim(); + + //Check the requestedSessionId is valid + response = client.GET(url + ";" + SessionHandler.__DefaultSessionIdPathParameterName + "=" + sessionId); + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + content = response.getContentAsString(); + assertThat(content, containsString("valid=true")); + } + finally + { + server.stop(); + } + } + public static class TestServlet extends HttpServlet { private static final long serialVersionUID = 1L; @@ -386,6 +546,26 @@ public class SessionHandlerTest } } + public static class TestRequestedSessionIdServlet extends HttpServlet + { + private static final long serialVersionUID = 1L; + public String _id = null; + + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + PrintWriter writer = response.getWriter(); + writer.println("requestedId=" + request.getRequestedSessionId()); + writer.println("valid=" + request.isRequestedSessionIdValid()); + + if ("create".equals(request.getParameter("action"))) + { + HttpSession session = request.getSession(true); + writer.println("createdId=" + session.getId()); + } + } + } + public class MockSessionCache extends AbstractSessionCache { @@ -395,8 +575,9 @@ public class SessionHandlerTest } @Override - public void shutdown() + public ManagedSession doDelete(String key) { + return null; } @Override @@ -411,12 +592,6 @@ public class SessionHandlerTest return null; } - @Override - public ManagedSession doDelete(String key) - { - return null; - } - @Override public boolean doReplace(String id, ManagedSession oldValue, ManagedSession newValue) { @@ -429,6 +604,11 @@ public class SessionHandlerTest return null; } + @Override + public void shutdown() + { + } + @Override protected ManagedSession doComputeIfAbsent(String id, Function mappingFunction) { diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/SessionHandlerTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/SessionHandlerTest.java index f839954c176..29bf24a2167 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/SessionHandlerTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/SessionHandlerTest.java @@ -50,7 +50,6 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; -import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -87,6 +86,7 @@ public class SessionHandlerTest String pathInContext = request.getPathInfo(); String[] split = pathInContext.substring(1).split("/"); + String requestedSessionId = request.getRequestedSessionId(); HttpSession session = request.getSession(false); if (split.length > 0) @@ -135,6 +135,10 @@ public class SessionHandlerTest } StringBuilder out = new StringBuilder(); + out.append("requestedSessionId=" + requestedSessionId).append('\n'); + out.append("requestedSessionIdValid=" + request.isRequestedSessionIdValid()).append('\n'); + + if (session == null) out.append("No Session\n"); else @@ -309,8 +313,9 @@ public class SessionHandlerTest String setCookie = response.get("SET-COOKIE"); assertThat(setCookie, containsString("Path=/")); String content = response.getContent(); - assertThat(content, startsWith("Session=")); - String id = content.substring(content.indexOf('=') + 1, content.indexOf('\n')); + assertThat(content, containsString("Session=")); + String id = content.substring(content.indexOf("Session=") + 8); + id = id.trim(); assertThat(id, not(equalTo("oldCookieId"))); endPoint.addInput(""" @@ -326,6 +331,64 @@ public class SessionHandlerTest assertThat(content, containsString("Session=" + id)); } + @Test + public void testRequestedSessionIdFromCookie() throws Exception + { + _server.stop(); + _sessionHandler.setSessionPath(null); + _contextHandler.setContextPath("/"); + _server.start(); + + //test with no session cookie + LocalConnector.LocalEndPoint endPoint = _connector.connect(); + endPoint.addInput(""" + GET / HTTP/1.1 + Host: localhost + + """); + + HttpTester.Response response = HttpTester.parseResponse(endPoint.getResponse()); + assertThat(response.getStatus(), equalTo(200)); + assertThat(response.getContent(), containsString("No Session")); + assertThat(response.getContent(), containsString("requestedSessionIdValid=false")); + + //test with a cookie for non-existant session + endPoint.addInput(""" + GET / HTTP/1.1 + Host: localhost + Cookie: JSESSIONID=%s + + """.formatted("123456789")); + response = HttpTester.parseResponse(endPoint.getResponse()); + assertThat(response.getStatus(), equalTo(200)); + assertThat(response.getContent(), containsString("No Session")); + assertThat(response.getContent(), containsString("requestedSessionIdValid=false")); + + //Make a real session + endPoint.addInput(""" + GET /create HTTP/1.1 + Host: localhost + + """); + + response = HttpTester.parseResponse(endPoint.getResponse()); + assertThat(response.getStatus(), equalTo(200)); + String content = response.getContent(); + assertThat(content, containsString("Session=")); + String id = content.substring(content.indexOf("Session=") + 8); + id = id.trim(); + + //Check the requestedSessionId is valid + endPoint.addInput(""" + GET / HTTP/1.1 + Host: localhost + Cookie: JSESSIONID=%s + + """.formatted(id)); + response = HttpTester.parseResponse(endPoint.getResponse()); + assertThat(response.getContent(), containsString("requestedSessionIdValid=true")); + } + @Test public void testSetAttribute() throws Exception { @@ -339,8 +402,9 @@ public class SessionHandlerTest HttpTester.Response response = HttpTester.parseResponse(endPoint.getResponse()); assertThat(response.getStatus(), equalTo(200)); String content = response.getContent(); - assertThat(content, startsWith("Session=")); - String id = content.substring(content.indexOf('=') + 1, content.indexOf('\n')); + assertThat(content, containsString("Session=")); + String id = content.substring(content.indexOf("Session=") + 8); + id = id.trim(); endPoint.addInput(""" GET /set/attribute/value HTTP/1.1 @@ -380,7 +444,7 @@ public class SessionHandlerTest HttpTester.Response response = HttpTester.parseResponse(endPoint.getResponse()); assertThat(response.getStatus(), equalTo(200)); String content = response.getContent(); - assertThat(content, startsWith("Session=")); + assertThat(content, containsString("Session=")); String setCookie = response.get(HttpHeader.SET_COOKIE); String id = setCookie.substring(setCookie.indexOf("JSESSIONID=") + 11, setCookie.indexOf("; Path=/")); From 235c5086aa9f28f4021b12534bb71c1ab836913f Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 17 Aug 2023 20:25:41 -0500 Subject: [PATCH 31/31] Improved `MetaInfConfigurationTest` cases (#10282) + Establish baseline tests for MetaInfConfiguration in preparation for PRs that adjust lookup / scan behaviors of MetaInfConfiguration --- .../ee10/webapp/MetaInfConfiguration.java | 2 +- .../ee10/webapp/MetaInfConfigurationTest.java | 643 +++++++++++++++--- .../src/test/resources/web25.xml | 10 - .../src/test/resources/web31.xml | 11 - .../src/test/resources/web31false.xml | 11 - 5 files changed, 535 insertions(+), 142 deletions(-) delete mode 100644 jetty-ee10/jetty-ee10-webapp/src/test/resources/web25.xml delete mode 100644 jetty-ee10/jetty-ee10-webapp/src/test/resources/web31.xml delete mode 100644 jetty-ee10/jetty-ee10-webapp/src/test/resources/web31false.xml diff --git a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java index 51a14b2fffa..436127ed58c 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java +++ b/jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java @@ -447,7 +447,7 @@ public class MetaInfConfiguration extends AbstractConfiguration * Scan for META-INF/web-fragment.xml file in the given jar. * * @param context the context for the scan - * @param jar the jar resource to scan for fragements in + * @param jar the jar resource to scan for fragments in * @param cache the resource cache */ public void scanForFragment(WebAppContext context, Resource jar, ConcurrentHashMap cache) diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/MetaInfConfigurationTest.java b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/MetaInfConfigurationTest.java index 7a05e100a82..d86c797282f 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/MetaInfConfigurationTest.java +++ b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/MetaInfConfigurationTest.java @@ -13,153 +13,578 @@ package org.eclipse.jetty.ee10.webapp; +import java.io.IOException; +import java.net.URI; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.Files; import java.nio.file.Path; -import java.util.Arrays; -import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Set; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.toolchain.test.FS; +import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; +import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; +import org.eclipse.jetty.util.TypeUtil; +import org.eclipse.jetty.util.URIUtil; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.resource.FileSystemPool; import org.eclipse.jetty.util.resource.Resource; +import org.eclipse.jetty.util.resource.ResourceCollators; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; +import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.is; +@ExtendWith(WorkDirExtension.class) public class MetaInfConfigurationTest { - public class TestableMetaInfConfiguration extends MetaInfConfiguration + @BeforeEach + public void beforeEach() { - List _expectedContainerScanTypes; - List _expectedWebAppScanTypes; - int _invocationCount = 0; - - public TestableMetaInfConfiguration(List expectedContainerScanTypes, List expectedWebAppScanTypes) - { - _expectedContainerScanTypes = expectedContainerScanTypes; - _expectedWebAppScanTypes = expectedWebAppScanTypes; - } - - @Override - public void scanJars(WebAppContext context, Collection jars, boolean useCaches, List scanTypes) throws Exception - { - assertNotNull(scanTypes); - List expectedScanTypes = null; - switch (_invocationCount) - { - case 0: - { - expectedScanTypes = _expectedContainerScanTypes; - break; - } - case 1: - { - expectedScanTypes = _expectedWebAppScanTypes; - break; - } - default: - { - fail("Too many invocations"); - } - } - - ++_invocationCount; - - assertNotNull(expectedScanTypes); - assertTrue(expectedScanTypes.containsAll(scanTypes)); - assertEquals(expectedScanTypes.size(), scanTypes.size()); - } + assertThat(FileSystemPool.INSTANCE.mounts(), empty()); } - @Test - public void testScanTypes() throws Exception + @AfterEach + public void tearDown() { - Path web25 = MavenTestingUtils.getTestResourcePathFile("web25.xml"); - Path web31 = MavenTestingUtils.getTestResourcePathFile("web31.xml"); - Path web31false = MavenTestingUtils.getTestResourcePathFile("web31false.xml"); - - //test a 2.5 webapp will not look for fragments as manually configured - MetaInfConfiguration meta25 = new TestableMetaInfConfiguration(MetaInfConfiguration.__allScanTypes, - Arrays.asList(MetaInfConfiguration.METAINF_TLDS, MetaInfConfiguration.METAINF_RESOURCES)); - WebAppContext context25 = new WebAppContext(); - context25.setConfigurationDiscovered(false); - context25.getMetaData().setWebDescriptor(new WebDescriptor(context25.getResourceFactory().newResource(web25))); - context25.getContext().getServletContext().setEffectiveMajorVersion(2); - context25.getContext().getServletContext().setEffectiveMinorVersion(5); - meta25.preConfigure(context25); - - //test a 2.5 webapp will look for fragments as configurationDiscovered default true - MetaInfConfiguration meta25b = new TestableMetaInfConfiguration(MetaInfConfiguration.__allScanTypes, - MetaInfConfiguration.__allScanTypes); - WebAppContext context25b = new WebAppContext(); - context25b.getMetaData().setWebDescriptor(new WebDescriptor(context25b.getResourceFactory().newResource(web25))); - context25b.getContext().getServletContext().setEffectiveMajorVersion(2); - context25b.getContext().getServletContext().setEffectiveMinorVersion(5); - meta25b.preConfigure(context25b); - - //test a 3.x metadata-complete webapp will not look for fragments - MetaInfConfiguration meta31 = new TestableMetaInfConfiguration(MetaInfConfiguration.__allScanTypes, - Arrays.asList(MetaInfConfiguration.METAINF_TLDS, MetaInfConfiguration.METAINF_RESOURCES)); - WebAppContext context31 = new WebAppContext(); - context31.getMetaData().setWebDescriptor(new WebDescriptor(context31.getResourceFactory().newResource(web31))); - context31.getContext().getServletContext().setEffectiveMajorVersion(3); - context31.getContext().getServletContext().setEffectiveMinorVersion(1); - meta31.preConfigure(context31); - - //test a 3.x non metadata-complete webapp will look for fragments - MetaInfConfiguration meta31false = new TestableMetaInfConfiguration(MetaInfConfiguration.__allScanTypes, - MetaInfConfiguration.__allScanTypes); - WebAppContext context31false = new WebAppContext(); - context31false.setConfigurationDiscovered(true); - context31false.getMetaData().setWebDescriptor(new WebDescriptor(context31false.getResourceFactory().newResource(web31false))); - context31false.getContext().getServletContext().setEffectiveMajorVersion(3); - context31false.getContext().getServletContext().setEffectiveMinorVersion(1); - meta31false.preConfigure(context31false); + assertThat(FileSystemPool.INSTANCE.mounts(), empty()); } /** - * This test examines both the classpath and the module path to find - * container resources. - * NOTE: the behaviour of the surefire plugin 3.0.0.M2 is different in - * jetty-9.4.x to jetty-10.0.x (where we use module-info): in jetty-9.4.x, - * we can use the --add-module argument to put the foo-bar-janb.jar onto the - * module path, but this doesn't seem to work in jetty-10.0.x. So this test - * will find foo-bar.janb.jar on the classpath, and jetty-util from the module path. - * - * @throws Exception if the test fails + * Test of a MetaInf scan of a Servlet 2.5 webapp, where + * {@link WebAppContext#setConfigurationDiscovered(boolean)} set to {@code false}, + * thus not performing any Servlet 3.0+ discovery steps for {@code META-INF/web-fragment.xml}. + * Scanning for {@code META-INF/resources} is unaffected by configuration. */ @Test - public void testFindAndFilterContainerPathsJDK9() throws Exception + public void testScanServlet25ConfigurationDiscoveredOff(WorkDir workDir) throws Exception + { + Path webappDir = workDir.getEmptyPathDir(); + Path webinf = webappDir.resolve("WEB-INF"); + FS.ensureDirExists(webinf); + Path webxml = webinf.resolve("web.xml"); + + String web25 = """ + + + Test 2.5 WebApp + + """; + + Files.writeString(webxml, web25, StandardCharsets.UTF_8); + Path libDir = webinf.resolve("lib"); + FS.ensureDirExists(libDir); + Path fooFragmentJar = libDir.resolve("foo-fragment.jar"); + try (FileSystem jarfs = createNewJarFileSystem(fooFragmentJar)) + { + Path webfragment = jarfs.getPath("/META-INF/web-fragment.xml"); + FS.ensureDirExists(webfragment.getParent()); + Files.writeString(webfragment, "", StandardCharsets.UTF_8); + } + + Path barResourceJar = libDir.resolve("bar-resources.jar"); + try (FileSystem jarfs = createNewJarFileSystem(barResourceJar)) + { + Path resourcesDir = jarfs.getPath("/META-INF/resources"); + Files.createDirectories(resourcesDir); + Path testTxt = resourcesDir.resolve("test.txt"); + Files.writeString(testTxt, "Test", StandardCharsets.UTF_8); + } + + Path zedTldJar = libDir.resolve("zed-tlds.jar"); + try (FileSystem jarfs = createNewJarFileSystem(zedTldJar)) + { + Path tldFile = jarfs.getPath("/META-INF/zed.tld"); + Files.createDirectory(tldFile.getParent()); + Files.writeString(tldFile, "", StandardCharsets.UTF_8); + } + + WebAppContext context = new WebAppContext(); + context.setServer(new Server()); + try + { + context.setBaseResource(context.getResourceFactory().newResource(webappDir)); + context.getMetaData().setWebDescriptor(new WebDescriptor(context.getResourceFactory().newResource(webxml))); + context.setConfigurationDiscovered(false); // don't allow discovery of servlet 3.0+ features + context.getContext().getServletContext().setEffectiveMajorVersion(2); + context.getContext().getServletContext().setEffectiveMinorVersion(5); + + MetaInfConfiguration metaInfConfiguration = new MetaInfConfiguration(); + metaInfConfiguration.preConfigure(context); + + List discoveredWebInfResources = context.getMetaData().getWebInfResources(false) + .stream() + .sorted(ResourceCollators.byName(true)) + .map(Resource::getURI) + .map(URI::toASCIIString) + .toList(); + String[] expectedWebInfResources = { + fooFragmentJar.toUri().toASCIIString(), + barResourceJar.toUri().toASCIIString(), + zedTldJar.toUri().toASCIIString() + }; + assertThat("Discovered WEB-INF resources", discoveredWebInfResources, hasItems(expectedWebInfResources)); + + // Since this is Servlet 2.5, and we have configuration-discovered turned off, we shouldn't see any web fragments + Map fragmentMap = getDiscoveredMetaInfFragments(context); + assertThat("META-INF/web-fragment.xml discovered (servlet 2.5 and configuration-discovered turned off)", fragmentMap.size(), is(0)); + + // Even on Servlet 2.5, when we have configuration-discovered turned off, we should still see the META-INF/resources/ + Set resourceSet = getDiscoveredMetaInfResource(context); + assertThat(resourceSet.size(), is(1)); + List discoveredResources = resourceSet + .stream() + .map(Resource::getURI) + .map(URI::toASCIIString) + .toList(); + String[] expectedResources = { + URIUtil.toJarFileUri(barResourceJar.toUri()).toASCIIString() + "META-INF/resources/" + }; + assertThat("META-INF/resources discovered (servlet 2.5 and configuration-discovered turned off)", discoveredResources, hasItems(expectedResources)); + + // TLDs discovered + Set tldSet = getDiscoveredMetaInfTlds(context); + assertThat(tldSet.size(), is(1)); + List discoveredTlds = tldSet + .stream() + .map(URL::toExternalForm) + .toList(); + String[] expectedTlds = { + URIUtil.toJarFileUri(zedTldJar.toUri()).toASCIIString() + "META-INF/zed.tld" + }; + assertThat("Discovered TLDs", discoveredTlds, hasItems(expectedTlds)); + } + finally + { + LifeCycle.stop(context.getResourceFactory()); + } + } + + /** + * Test of a MetaInf scan of a Servlet 2.5 webapp, where + * {@link WebAppContext#setConfigurationDiscovered(boolean)} is left at default (@{code true}) + * allowing the performing of Servlet 3.0+ discovery steps for {@code META-INF/web-fragment.xml} and {@code META-INF/resources} + */ + @Test + public void testScanServlet25ConfigurationDiscoveredDefault(WorkDir workDir) throws Exception + { + Path webappDir = workDir.getEmptyPathDir(); + Path webinf = webappDir.resolve("WEB-INF"); + FS.ensureDirExists(webinf); + Path webxml = webinf.resolve("web.xml"); + + String web25 = """ + + + Test 2.5 WebApp + + """; + + Files.writeString(webxml, web25, StandardCharsets.UTF_8); + Path libDir = webinf.resolve("lib"); + FS.ensureDirExists(libDir); + Path fooFragmentJar = libDir.resolve("foo-fragment.jar"); + try (FileSystem jarfs = createNewJarFileSystem(fooFragmentJar)) + { + Path webfragment = jarfs.getPath("/META-INF/web-fragment.xml"); + FS.ensureDirExists(webfragment.getParent()); + Files.writeString(webfragment, "", StandardCharsets.UTF_8); + } + + Path barResourceJar = libDir.resolve("bar-resources.jar"); + try (FileSystem jarfs = createNewJarFileSystem(barResourceJar)) + { + Path resourcesDir = jarfs.getPath("/META-INF/resources"); + Files.createDirectories(resourcesDir); + Path testTxt = resourcesDir.resolve("test.txt"); + Files.writeString(testTxt, "Test", StandardCharsets.UTF_8); + } + + Path zedTldJar = libDir.resolve("zed-tlds.jar"); + try (FileSystem jarfs = createNewJarFileSystem(zedTldJar)) + { + Path tldFile = jarfs.getPath("/META-INF/zed.tld"); + Files.createDirectory(tldFile.getParent()); + Files.writeString(tldFile, "", StandardCharsets.UTF_8); + } + + WebAppContext context = new WebAppContext(); + context.setServer(new Server()); + try + { + context.setBaseResource(context.getResourceFactory().newResource(webappDir)); + context.getMetaData().setWebDescriptor(new WebDescriptor(context.getResourceFactory().newResource(webxml))); + // context25.setConfigurationDiscovered(true); // The default value + context.getContext().getServletContext().setEffectiveMajorVersion(2); + context.getContext().getServletContext().setEffectiveMinorVersion(5); + + MetaInfConfiguration metaInfConfiguration = new MetaInfConfiguration(); + metaInfConfiguration.preConfigure(context); + + List discoveredWebInfResources = context.getMetaData().getWebInfResources(false) + .stream() + .sorted(ResourceCollators.byName(true)) + .map(Resource::getURI) + .map(URI::toASCIIString) + .toList(); + String[] expectedWebInfResources = { + fooFragmentJar.toUri().toASCIIString(), + barResourceJar.toUri().toASCIIString(), + zedTldJar.toUri().toASCIIString() + }; + assertThat("Discovered WEB-INF resources", discoveredWebInfResources, hasItems(expectedWebInfResources)); + + // Since this is Servlet 2.5, and we have configuration-discovered turned on, we should see the META-INF/web-fragment.xml entries + Map fragmentMap = getDiscoveredMetaInfFragments(context); + assertThat(fragmentMap.size(), is(1)); + List discoveredFragments = fragmentMap.entrySet() + .stream() + .map(e -> e.getValue().getURI().toASCIIString()) + .toList(); + String[] expectedFragments = { + URIUtil.toJarFileUri(fooFragmentJar.toUri()).toASCIIString() + "META-INF/web-fragment.xml" + }; + assertThat("META-INF/web-fragment.xml discovered (servlet 2.5 and configuration-discovered=true)", discoveredFragments, hasItems(expectedFragments)); + + // Since this is Servlet 2.5, and we have configuration-discovered turned on, we should see the META-INF/resources/ + Set resourceSet = getDiscoveredMetaInfResource(context); + assertThat(resourceSet.size(), is(1)); + List discoveredResources = resourceSet + .stream() + .map(Resource::getURI) + .map(URI::toASCIIString) + .toList(); + String[] expectedResources = { + URIUtil.toJarFileUri(barResourceJar.toUri()).toASCIIString() + "META-INF/resources/" + }; + assertThat("META-INF/resources discovered (servlet 2.5 and configuration-discovered=true)", discoveredResources, hasItems(expectedResources)); + + // TLDs discovered + Set tldSet = getDiscoveredMetaInfTlds(context); + assertThat(tldSet.size(), is(1)); + List discoveredTlds = tldSet + .stream() + .map(URL::toExternalForm) + .toList(); + String[] expectedTlds = { + URIUtil.toJarFileUri(zedTldJar.toUri()).toASCIIString() + "META-INF/zed.tld" + }; + assertThat("Discovered TLDs", discoveredTlds, hasItems(expectedTlds)); + } + finally + { + LifeCycle.stop(context.getResourceFactory()); + } + } + + /** + * Test of a MetaInf scan of a Servlet 3.0 webapp, metadata-complete is set to {@code false} + */ + @Test + public void testScanServlet30MetadataCompleteFalse(WorkDir workDir) throws Exception + { + Path webappDir = workDir.getEmptyPathDir(); + Path webinf = webappDir.resolve("WEB-INF"); + FS.ensureDirExists(webinf); + Path webxml = webinf.resolve("web.xml"); + + String web30 = """ + + + Test 3.0 WebApp + + """; + + Files.writeString(webxml, web30, StandardCharsets.UTF_8); + Path libDir = webinf.resolve("lib"); + FS.ensureDirExists(libDir); + Path fooFragmentJar = libDir.resolve("foo-fragment.jar"); + try (FileSystem jarfs = createNewJarFileSystem(fooFragmentJar)) + { + Path webfragment = jarfs.getPath("/META-INF/web-fragment.xml"); + FS.ensureDirExists(webfragment.getParent()); + Files.writeString(webfragment, "", StandardCharsets.UTF_8); + } + + Path barResourceJar = libDir.resolve("bar-resources.jar"); + try (FileSystem jarfs = createNewJarFileSystem(barResourceJar)) + { + Path resourcesDir = jarfs.getPath("/META-INF/resources"); + Files.createDirectories(resourcesDir); + Path testTxt = resourcesDir.resolve("test.txt"); + Files.writeString(testTxt, "Test", StandardCharsets.UTF_8); + } + + Path zedTldJar = libDir.resolve("zed-tlds.jar"); + try (FileSystem jarfs = createNewJarFileSystem(zedTldJar)) + { + Path tldFile = jarfs.getPath("/META-INF/zed.tld"); + Files.createDirectory(tldFile.getParent()); + Files.writeString(tldFile, "", StandardCharsets.UTF_8); + } + + WebAppContext context = new WebAppContext(); + context.setServer(new Server()); + try + { + context.setBaseResource(context.getResourceFactory().newResource(webappDir)); + context.getMetaData().setWebDescriptor(new WebDescriptor(context.getResourceFactory().newResource(webxml))); + // context25.setConfigurationDiscovered(true); // The default value + context.getContext().getServletContext().setEffectiveMajorVersion(3); + context.getContext().getServletContext().setEffectiveMinorVersion(0); + + MetaInfConfiguration metaInfConfiguration = new MetaInfConfiguration(); + metaInfConfiguration.preConfigure(context); + + List discoveredWebInfResources = context.getMetaData().getWebInfResources(false) + .stream() + .sorted(ResourceCollators.byName(true)) + .map(Resource::getURI) + .map(URI::toASCIIString) + .toList(); + String[] expectedWebInfResources = { + fooFragmentJar.toUri().toASCIIString(), + barResourceJar.toUri().toASCIIString(), + zedTldJar.toUri().toASCIIString() + }; + assertThat("Discovered WEB-INF resources", discoveredWebInfResources, hasItems(expectedWebInfResources)); + + // Since this is Servlet 3.0, and we have configuration-discovered turned on, we should see the META-INF/web-fragment.xml entries + Map fragmentMap = getDiscoveredMetaInfFragments(context); + assertThat(fragmentMap.size(), is(1)); + List discoveredFragments = fragmentMap.entrySet() + .stream() + .map(e -> e.getValue().getURI().toASCIIString()) + .toList(); + String[] expectedFragments = { + URIUtil.toJarFileUri(fooFragmentJar.toUri()).toASCIIString() + "META-INF/web-fragment.xml" + }; + assertThat("META-INF/web-fragment.xml discovered (servlet 3.0, and metadata-complete=false, and configuration-discovered=true)", discoveredFragments, hasItems(expectedFragments)); + + // Since this is Servlet 3.0, and we have configuration-discovered turned on, we should see the META-INF/resources/ + Set resourceSet = getDiscoveredMetaInfResource(context); + assertThat(resourceSet.size(), is(1)); + List discoveredResources = resourceSet + .stream() + .map(Resource::getURI) + .map(URI::toASCIIString) + .toList(); + String[] expectedResources = { + URIUtil.toJarFileUri(barResourceJar.toUri()).toASCIIString() + "META-INF/resources/" + }; + assertThat("META-INF/resources discovered (servlet 3.0, and metadata-complete=false, and configuration-discovered=true)", discoveredResources, hasItems(expectedResources)); + + // TLDs discovered + Set tldSet = getDiscoveredMetaInfTlds(context); + assertThat(tldSet.size(), is(1)); + List discoveredTlds = tldSet + .stream() + .map(URL::toExternalForm) + .toList(); + String[] expectedTlds = { + URIUtil.toJarFileUri(zedTldJar.toUri()).toASCIIString() + "META-INF/zed.tld" + }; + assertThat("Discovered TLDs", discoveredTlds, hasItems(expectedTlds)); + } + finally + { + LifeCycle.stop(context.getResourceFactory()); + } + } + + /** + * Test of a MetaInf scan of a Servlet 3.1 webapp, metadata-complete is set to {@code true} + */ + @Test + public void testScanServlet31MetadataCompleteTrue(WorkDir workDir) throws Exception + { + Path webappDir = workDir.getEmptyPathDir(); + Path webinf = webappDir.resolve("WEB-INF"); + FS.ensureDirExists(webinf); + Path webxml = webinf.resolve("web.xml"); + + String web31 = """ + + + Test 3.1 WebApp + + """; + + Files.writeString(webxml, web31, StandardCharsets.UTF_8); + Path libDir = webinf.resolve("lib"); + FS.ensureDirExists(libDir); + Path fooFragmentJar = libDir.resolve("foo-fragment.jar"); + try (FileSystem jarfs = createNewJarFileSystem(fooFragmentJar)) + { + Path webfragment = jarfs.getPath("/META-INF/web-fragment.xml"); + FS.ensureDirExists(webfragment.getParent()); + Files.writeString(webfragment, "", StandardCharsets.UTF_8); + } + + Path barResourceJar = libDir.resolve("bar-resources.jar"); + try (FileSystem jarfs = createNewJarFileSystem(barResourceJar)) + { + Path resourcesDir = jarfs.getPath("/META-INF/resources"); + Files.createDirectories(resourcesDir); + Path testTxt = resourcesDir.resolve("test.txt"); + Files.writeString(testTxt, "Test", StandardCharsets.UTF_8); + } + + Path zedTldJar = libDir.resolve("zed-tlds.jar"); + try (FileSystem jarfs = createNewJarFileSystem(zedTldJar)) + { + Path tldFile = jarfs.getPath("/META-INF/zed.tld"); + Files.createDirectory(tldFile.getParent()); + Files.writeString(tldFile, "", StandardCharsets.UTF_8); + } + + WebAppContext context = new WebAppContext(); + context.setServer(new Server()); + try + { + context.setBaseResource(context.getResourceFactory().newResource(webappDir)); + context.getMetaData().setWebDescriptor(new WebDescriptor(context.getResourceFactory().newResource(webxml))); + context.getContext().getServletContext().setEffectiveMajorVersion(3); + context.getContext().getServletContext().setEffectiveMinorVersion(1); + + MetaInfConfiguration metaInfConfiguration = new MetaInfConfiguration(); + metaInfConfiguration.preConfigure(context); + + List discoveredWebInfResources = context.getMetaData().getWebInfResources(false) + .stream() + .sorted(ResourceCollators.byName(true)) + .map(Resource::getURI) + .map(URI::toASCIIString) + .toList(); + String[] expectedWebInfResources = { + fooFragmentJar.toUri().toASCIIString(), + barResourceJar.toUri().toASCIIString(), + zedTldJar.toUri().toASCIIString() + }; + assertThat("Discovered WEB-INF resources", discoveredWebInfResources, hasItems(expectedWebInfResources)); + + // Since this is Servlet 3.1, and we have metadata-complete=true, we should see no fragments + Map fragmentMap = getDiscoveredMetaInfFragments(context); + assertThat("META-INF/web-fragment.xml discovered (servlet 3.1, and metadata-complete=true)", fragmentMap.size(), is(0)); + + // Even on Servlet 3.1, with metadata-complete=true, we should still see the META-INF/resources/ + Set resourceSet = getDiscoveredMetaInfResource(context); + assertThat(resourceSet.size(), is(1)); + List discoveredResources = resourceSet + .stream() + .map(Resource::getURI) + .map(URI::toASCIIString) + .toList(); + String[] expectedResources = { + URIUtil.toJarFileUri(barResourceJar.toUri()).toASCIIString() + "META-INF/resources/" + }; + assertThat("META-INF/resources discovered (servlet 3.1 and metadata-complete=true)", discoveredResources, hasItems(expectedResources)); + + // TLDs discovered + Set tldSet = getDiscoveredMetaInfTlds(context); + assertThat(tldSet.size(), is(1)); + List discoveredTlds = tldSet + .stream() + .map(URL::toExternalForm) + .toList(); + String[] expectedTlds = { + URIUtil.toJarFileUri(zedTldJar.toUri()).toASCIIString() + "META-INF/zed.tld" + }; + assertThat("Discovered TLDs", discoveredTlds, hasItems(expectedTlds)); + } + finally + { + LifeCycle.stop(context.getResourceFactory()); + } + } + + private FileSystem createNewJarFileSystem(Path jarFile) throws IOException + { + Map env = new HashMap<>(); + env.put("create", "true"); + URI jarUri = URIUtil.uriJarPrefix(jarFile.toUri(), "!/"); + return FileSystems.newFileSystem(jarUri, env); + } + + /** + * This test examines both the classpath and the module path to find container resources. + * This test looks {@code foo-bar.janb.jar} on the classpath (it was added there by the surefire configuration + * present in the {@code pom.xml}), and the {@code servlet-api} from the module path. + */ + @Test + public void testGetContainerPathsWithModuleSystem() throws Exception { MetaInfConfiguration config = new MetaInfConfiguration(); WebAppContext context = new WebAppContext(); context.setServer(new Server()); - config.preConfigure(context); try { context.setAttribute(MetaInfConfiguration.CONTAINER_JAR_PATTERN, ".*servlet-api-[^/]*\\.jar$|.*/foo-bar-janb.jar"); WebAppClassLoader loader = new WebAppClassLoader(context); context.setClassLoader(loader); - config.findAndFilterContainerPaths(context); - List containerResources = context.getMetaData().getContainerResources(); + config.preConfigure(context); - assertEquals(2, containerResources.size()); - for (Resource r : containerResources) - { - String s = r.toString(); - assertTrue(s.endsWith("foo-bar-janb.jar") || s.contains("servlet-api")); - } + Class janbClazz = Class.forName("foo.bar.janb.What", false, loader); + URI janbUri = TypeUtil.getLocationOfClass(janbClazz); + Class servletClazz = Class.forName("jakarta.servlet.Servlet", false, loader); + URI servletUri = TypeUtil.getLocationOfClass(servletClazz); + + List discoveredContainerResources = context.getMetaData().getContainerResources() + .stream() + .sorted(ResourceCollators.byName(true)) + .map(Resource::getURI) + .map(URI::toASCIIString) + .toList(); + // we "correct" the bad file URLs that come from the ClassLoader + // to be the same as what comes from every non-classloader URL/URI. + String[] expectedContainerResources = { + URIUtil.correctFileURI(janbUri).toASCIIString(), + URIUtil.correctFileURI(servletUri).toASCIIString() + }; + assertThat("Discovered Container resources", discoveredContainerResources, hasItems(expectedContainerResources)); } finally { config.postConfigure(context); + // manually stop ResourceFactory. + // normally this would be done via WebAppContext.stop(), but we didn't start the context. + LifeCycle.stop(context.getResourceFactory()); } } + + private Map getDiscoveredMetaInfFragments(WebAppContext context) + { + return (Map)context.getAttribute(MetaInfConfiguration.METAINF_FRAGMENTS); + } + + private Set getDiscoveredMetaInfResource(WebAppContext context) + { + return (Set)context.getAttribute(MetaInfConfiguration.METAINF_RESOURCES); + } + + private Set getDiscoveredMetaInfTlds(WebAppContext context) + { + return (Set)context.getAttribute(MetaInfConfiguration.METAINF_TLDS); + } } diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/resources/web25.xml b/jetty-ee10/jetty-ee10-webapp/src/test/resources/web25.xml deleted file mode 100644 index da2e65b6007..00000000000 --- a/jetty-ee10/jetty-ee10-webapp/src/test/resources/web25.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - Test 2.5 WebApp - - diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/resources/web31.xml b/jetty-ee10/jetty-ee10-webapp/src/test/resources/web31.xml deleted file mode 100644 index 768c5ea212f..00000000000 --- a/jetty-ee10/jetty-ee10-webapp/src/test/resources/web31.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - Test 31 WebApp - - diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/resources/web31false.xml b/jetty-ee10/jetty-ee10-webapp/src/test/resources/web31false.xml deleted file mode 100644 index 81750df9dda..00000000000 --- a/jetty-ee10/jetty-ee10-webapp/src/test/resources/web31false.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - Test 31 WebApp - -