This is an automated email from the ASF dual-hosted git repository. cziegeler pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/felix-dev.git
The following commit(s) were added to refs/heads/master by this push: new 485c095e94 FELIX-6707 : Fix uriComplianceMode 485c095e94 is described below commit 485c095e94e4fe7522521b28f82a681fdb0d598f Author: Paul <p...@blueconic.com> AuthorDate: Wed May 8 09:41:40 2024 +0200 FELIX-6707 : Fix uriComplianceMode * Fix uriComplianceMode When making code review changes, an error was made in using the enum. This pr fixes it. Will add a unit test to guard behavior. * Add unit tests to assert org.eclipse.jetty.UriComplianceMode works * Assert response --- .../felix/http/jetty/internal/JettyService.java | 13 ++- .../jetty/it/JettyUriComplianceModeDefaultIT.java | 114 +++++++++++++++++++++ .../jetty/it/JettyUriComplianceModeLegacyIT.java | 83 +++++++++++++++ 3 files changed, 205 insertions(+), 5 deletions(-) diff --git a/http/jetty12/src/main/java/org/apache/felix/http/jetty/internal/JettyService.java b/http/jetty12/src/main/java/org/apache/felix/http/jetty/internal/JettyService.java index b52adc5a5b..6c483c47bb 100644 --- a/http/jetty12/src/main/java/org/apache/felix/http/jetty/internal/JettyService.java +++ b/http/jetty12/src/main/java/org/apache/felix/http/jetty/internal/JettyService.java @@ -16,6 +16,10 @@ */ package org.apache.felix.http.jetty.internal; +import static org.eclipse.jetty.http.UriCompliance.LEGACY; +import static org.eclipse.jetty.http.UriCompliance.UNAMBIGUOUS; +import static org.eclipse.jetty.http.UriCompliance.UNSAFE; + import java.net.Inet4Address; import java.net.Inet6Address; import java.net.InetAddress; @@ -677,15 +681,14 @@ public final class JettyService config.setRequestHeaderSize(this.config.getHeaderSize()); config.setResponseHeaderSize(this.config.getHeaderSize()); config.setOutputBufferSize(this.config.getResponseBufferSize()); - + String uriComplianceMode = this.config.getProperty(JettyConfig.FELIX_JETTY_URI_COMPLIANCE_MODE, null); if (uriComplianceMode != null) { try { - config.setUriCompliance(UriCompliance.valueOf(uriComplianceMode)); + UriCompliance compliance = UriCompliance.valueOf(uriComplianceMode); + config.setUriCompliance(compliance); - if (UriCompliance.LEGACY.equals(uriComplianceMode) - || UriCompliance.UNSAFE.equals(uriComplianceMode) - || UriCompliance.UNAMBIGUOUS.equals(uriComplianceMode)) { + if (LEGACY.equals(compliance) || UNSAFE.equals(compliance) || UNAMBIGUOUS.equals(compliance)) { // See https://github.com/jetty/jetty.project/issues/11448#issuecomment-1969206031 this.server.getContainedBeans(ServletHandler.class) .forEach(handler -> handler.setDecodeAmbiguousURIs(true)); diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettyUriComplianceModeDefaultIT.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettyUriComplianceModeDefaultIT.java new file mode 100644 index 0000000000..591aa51e5b --- /dev/null +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettyUriComplianceModeDefaultIT.java @@ -0,0 +1,114 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.felix.http.jetty.it; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.ops4j.pax.exam.CoreOptions.mavenBundle; +import static org.ops4j.pax.exam.cm.ConfigurationAdminOptions.newConfiguration; + +import java.io.IOException; +import java.net.URI; +import java.util.Hashtable; +import java.util.Map; + +import javax.inject.Inject; +import jakarta.servlet.Servlet; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.client.ContentResponse; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.transport.HttpClientTransportOverHTTP; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.ops4j.pax.exam.Option; +import org.ops4j.pax.exam.junit.PaxExam; +import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy; +import org.ops4j.pax.exam.spi.reactors.PerClass; +import org.osgi.framework.BundleContext; +import org.osgi.service.http.HttpService; +import org.osgi.service.servlet.whiteboard.HttpWhiteboardConstants; +@RunWith(PaxExam.class) +@ExamReactorStrategy(PerClass.class) +public class JettyUriComplianceModeDefaultIT extends AbstractJettyTestSupport { + + @Inject + protected BundleContext bundleContext; + + @Override + protected Option[] additionalOptions() throws IOException { + String jettyVersion = System.getProperty("jetty.version", "12.0.8"); + return new Option[] { + spifly(), + + // bundles for the server side + mavenBundle().groupId("org.eclipse.jetty.ee10").artifactId("jetty-ee10-webapp").version(jettyVersion), + mavenBundle().groupId("org.eclipse.jetty.ee10").artifactId("jetty-ee10-servlet").version(jettyVersion), + mavenBundle().groupId("org.eclipse.jetty").artifactId("jetty-xml").version(jettyVersion), + + // additional bundles for the client side + mavenBundle().groupId("org.eclipse.jetty").artifactId("jetty-alpn-client").version(jettyVersion), + mavenBundle().groupId("org.eclipse.jetty").artifactId("jetty-client").version(jettyVersion) + }; + } + + @Override + protected Option felixHttpConfig(int httpPort) { + return newConfiguration("org.apache.felix.http") + .put("org.osgi.service.http.port", httpPort) + .asOption(); + } + + @Before + public void setup(){ + assertNotNull(bundleContext); + bundleContext.registerService(Servlet.class, new UriComplianceEndpoint(), new Hashtable<>(Map.of( + HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN, "/*" + ))); + } + + @Test + public void testUriCompliance() throws Exception { + HttpClientTransportOverHTTP transport = new HttpClientTransportOverHTTP(); + HttpClient httpClient = new HttpClient(transport); + httpClient.start(); + + Object value = bundleContext.getServiceReference(HttpService.class).getProperty("org.osgi.service.http.port"); + int httpPort = Integer.parseInt((String) value); + + URI destUriWorking = new URI(String.format("http://localhost:%d/endpoint/working", httpPort)); + URI destUriAmbigousPath = new URI("http://localhost:" + httpPort + "/endpoint/ambigousPathitem_0_http%3A%2F%2Fwww.test.com%2F0.html/abc"); + + ContentResponse response = httpClient.GET(destUriWorking); + assertEquals(200, response.getStatus()); + assertEquals("OK", response.getContentAsString()); + + // blocked with HTTP 400 by default + assertEquals(400, httpClient.GET(destUriAmbigousPath).getStatus()); + } + + static final class UriComplianceEndpoint extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + resp.setStatus(200); + resp.getWriter().write("OK"); + } + } +} \ No newline at end of file diff --git a/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettyUriComplianceModeLegacyIT.java b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettyUriComplianceModeLegacyIT.java new file mode 100644 index 0000000000..7da80d0573 --- /dev/null +++ b/http/jetty12/src/test/java/org/apache/felix/http/jetty/it/JettyUriComplianceModeLegacyIT.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.felix.http.jetty.it; + +import static org.eclipse.jetty.http.UriCompliance.LEGACY; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.ops4j.pax.exam.CoreOptions.mavenBundle; +import static org.ops4j.pax.exam.cm.ConfigurationAdminOptions.newConfiguration; + +import java.io.IOException; +import java.net.URI; +import java.util.Hashtable; +import java.util.Map; + +import javax.inject.Inject; +import jakarta.servlet.Servlet; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.client.ContentResponse; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.transport.HttpClientTransportOverHTTP; +import org.eclipse.jetty.http.UriCompliance; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.ops4j.pax.exam.Option; +import org.ops4j.pax.exam.junit.PaxExam; +import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy; +import org.ops4j.pax.exam.spi.reactors.PerClass; +import org.osgi.framework.BundleContext; +import org.osgi.service.http.HttpService; +import org.osgi.service.servlet.whiteboard.HttpWhiteboardConstants; + +@RunWith(PaxExam.class) +@ExamReactorStrategy(PerClass.class) +public class JettyUriComplianceModeLegacyIT extends JettyUriComplianceModeDefaultIT { + + @Override + protected Option felixHttpConfig(int httpPort) { + return newConfiguration("org.apache.felix.http") + .put("org.osgi.service.http.port", httpPort) + .put("org.eclipse.jetty.UriComplianceMode", LEGACY.getName()) + .asOption(); + } + + @Test + public void testUriCompliance() throws Exception { + HttpClientTransportOverHTTP transport = new HttpClientTransportOverHTTP(); + HttpClient httpClient = new HttpClient(transport); + httpClient.start(); + + Object value = bundleContext.getServiceReference(HttpService.class).getProperty("org.osgi.service.http.port"); + int httpPort = Integer.parseInt((String) value); + + URI destUriWorking = new URI(String.format("http://localhost:%d/endpoint/working", httpPort)); + URI destUriAmbigousPath = new URI("http://localhost:" + httpPort + "/endpoint/ambigousPathitem_0_http%3A%2F%2Fwww.test.com%2F0.html/abc"); + + ContentResponse response = httpClient.GET(destUriWorking); + assertEquals(200, response.getStatus()); + assertEquals("OK", response.getContentAsString()); + + // no longer blocked due to LEGACY compliance mode + ContentResponse response2 = httpClient.GET(destUriAmbigousPath); + assertEquals(200, response2.getStatus()); + assertEquals("OK", response2.getContentAsString()); + } +} \ No newline at end of file