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

Reply via email to