fix: return url without port if default port protocol is used (#1511)

* fix: return url without port if default port protocol is used * tests: set server port to 8080 and assert that all generated url use port 8080
eclipse · Jan 31, 2024 · a5dda8c · a5dda8c
1 parent 537a942
commit a5dda8c
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 11 deletions.
diff --git a/hawkbit-core/src/main/java/org/eclipse/hawkbit/api/PropertyBasedArtifactUrlHandler.java b/hawkbit-core/src/main/java/org/eclipse/hawkbit/api/PropertyBasedArtifactUrlHandler.java
@@ -102,7 +102,7 @@ private String generateUrl(final UrlProtocol protocol, final URLPlaceholder plac
         String urlPattern = protocol.getRef();
 
         for (final Entry<String, String> entry : entrySet) {
-            if (entry.getKey().equals(PORT_PLACEHOLDER)) {
+            if (List.of(PORT_PLACEHOLDER,PORT_REQUEST_PLACEHOLDER).contains(entry.getKey())) {
                 urlPattern = urlPattern.replace(":{" + entry.getKey() + "}",
                         ObjectUtils.isEmpty(entry.getValue()) ? "" : (":" + entry.getValue()));
             } else {
@@ -157,8 +157,8 @@ private static String getRequestPort(final UrlProtocol protocol, final URI reque
         if (requestUri == null) {
             return getPort(protocol);
         }
-
-        return requestUri.getPort() > 0 ? String.valueOf(requestUri.getPort()) : getPort(protocol);
+        // if port undefined then default protocol port is used
+        return requestUri.getPort() > 0 ? String.valueOf(requestUri.getPort()) : "";
     }
 
     private static String getRequestHost(final UrlProtocol protocol, final URI requestUri) {

diff --git a/hawkbit-core/src/test/java/org/eclipse/hawkbit/api/PropertyBasedArtifactUrlHandlerTest.java b/hawkbit-core/src/test/java/org/eclipse/hawkbit/api/PropertyBasedArtifactUrlHandlerTest.java
@@ -181,6 +181,24 @@ public void urlGenerationWithPortFromRequest() throws URISyntaxException {
                         + SOFTWAREMODULEID + "/artifacts/" + FILENAME_ENCODE));
     }
 
+    @Test
+    @Description("Verfies that if default protocol port in request is used then url is returned without port")
+    public void urlGenerationWithPortFromRequestForHttps() throws URISyntaxException {
+        String protocol = "https";
+        final UrlProtocol proto = new UrlProtocol();
+        proto.setRef(
+                "{protocolRequest}://{hostnameRequest}:{portRequest}/{tenant}/controller/v1/{controllerId}/softwaremodules/{softwareModuleId}/artifacts/{artifactFileName}");
+        proto.setProtocol(protocol);
+        properties.getProtocols().put("download-http", proto);
+
+        URI uri = new URI(protocol+"://anotherHost.com");
+        final List<ArtifactUrl> urls = urlHandlerUnderTest.getUrls(placeholder, ApiType.DDI, uri);
+        assertThat(urls).containsExactly(new ArtifactUrl(protocol.toUpperCase(), "download-http",
+                uri +"/" + TENANT + "/controller/v1/" + CONTROLLER_ID + "/softwaremodules/"
+                        + SOFTWAREMODULEID + "/artifacts/" + FILENAME_ENCODE));
+
+    }
+
     @Test
     @Description("Verfies that the domain of the statically defined hostname is replaced with the domain of the request.")
     public void urlGenerationWithDomainFromRequest() throws URISyntaxException {

diff --git a/...ce/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/AbstractDDiApiIntegrationTest.java b/...ce/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/AbstractDDiApiIntegrationTest.java
@@ -61,7 +61,8 @@
 @TestPropertySource(locations = "classpath:/ddi-test.properties")
 public abstract class AbstractDDiApiIntegrationTest extends AbstractRestIntegrationTest {
 
-    protected static final String HTTP_LOCALHOST = "http://localhost:8080/";
+    public static final int HTTP_PORT = 8080;
+    protected static final String HTTP_LOCALHOST = String.format("http://localhost:%s/",HTTP_PORT);
     protected static final String CONTROLLER_BASE = "/{tenant}/controller/v1/{controllerId}";
 
     protected static final String SOFTWARE_MODULE_ARTIFACTS = CONTROLLER_BASE
@@ -156,7 +157,8 @@ protected ResultActions postCancelFeedback(final MediaType mediaType, final Stri
 
     protected ResultActions performGet(final String url, final MediaType mediaType, final ResultMatcher statusMatcher,
             final String... values) throws Exception {
-        return mvc.perform(MockMvcRequestBuilders.get(url, values).accept(mediaType))
+        return mvc.perform(MockMvcRequestBuilders.get(url, values).accept(mediaType)
+                .with(new RequestOnHawkbitDefaultPortPostProcessor()))
                 .andDo(MockMvcResultPrinter.print()).andExpect(statusMatcher)
                 .andExpect(content().contentTypeCompatibleWith(mediaType));
     }
@@ -242,12 +244,12 @@ private ResultActions verifyBasePayload(final String prefix, final ResultActions
     }
 
     protected String installedBaseLink(final String controllerId, final String actionId) {
-        return "http://localhost/" + tenantAware.getCurrentTenant() + "/controller/v1/" + controllerId
+        return HTTP_LOCALHOST + tenantAware.getCurrentTenant() + "/controller/v1/" + controllerId
                 + "/installedBase/" + actionId;
     }
 
     protected String deploymentBaseLink(final String controllerId, final String actionId) {
-        return "http://localhost/" + tenantAware.getCurrentTenant() + "/controller/v1/" + controllerId
+        return HTTP_LOCALHOST + tenantAware.getCurrentTenant() + "/controller/v1/" + controllerId
                 + "/deploymentBase/" + actionId;
     }
 

diff --git a/...i-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java b/...i-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java
@@ -254,7 +254,7 @@ void rootRsNotModified() throws Exception {
                 .getContent().get(0);
         final String etagWithFirstUpdate = mvc
                 .perform(get(CONTROLLER_BASE, tenantAware.getCurrentTenant(), controllerId)
-                        .header("If-None-Match", etag).accept(MediaType.APPLICATION_JSON))
+                        .header("If-None-Match", etag).accept(MediaType.APPLICATION_JSON).with(new RequestOnHawkbitDefaultPortPostProcessor()))
                 .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk())
                 .andExpect(content().contentType(MediaType.APPLICATION_JSON))
                 .andExpect(jsonPath("$.config.polling.sleep", equalTo("00:01:00")))
@@ -266,7 +266,8 @@ void rootRsNotModified() throws Exception {
         assertThat(etagWithFirstUpdate).isNotNull();
 
         mvc.perform(get(CONTROLLER_BASE, tenantAware.getCurrentTenant(), controllerId).header("If-None-Match",
-                etagWithFirstUpdate)).andDo(MockMvcResultPrinter.print()).andExpect(status().isNotModified());
+                etagWithFirstUpdate).with(new RequestOnHawkbitDefaultPortPostProcessor()))
+                .andDo(MockMvcResultPrinter.print()).andExpect(status().isNotModified());
 
         // now lets finish the update
         sendDeploymentActionFeedback(target, updateAction, "closed", null).andDo(MockMvcResultPrinter.print())
@@ -276,7 +277,8 @@ void rootRsNotModified() throws Exception {
         // original state cannot be restored
         final String etagAfterInstallation = mvc
                 .perform(get(CONTROLLER_BASE, tenantAware.getCurrentTenant(), controllerId)
-                        .header("If-None-Match", etag).accept(MediaType.APPLICATION_JSON))
+                        .header("If-None-Match", etag).accept(MediaType.APPLICATION_JSON)
+                        .with(new RequestOnHawkbitDefaultPortPostProcessor()))
                 .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk())
                 .andExpect(content().contentType(MediaType.APPLICATION_JSON))
                 .andExpect(jsonPath("$.config.polling.sleep", equalTo("00:01:00")))
@@ -294,7 +296,8 @@ void rootRsNotModified() throws Exception {
                 .getContent().get(0);
 
         mvc.perform(get(CONTROLLER_BASE, tenantAware.getCurrentTenant(), controllerId)
-                .header("If-None-Match", etagAfterInstallation).accept(MediaType.APPLICATION_JSON))
+                .header("If-None-Match", etagAfterInstallation).accept(MediaType.APPLICATION_JSON)
+                .with(new RequestOnHawkbitDefaultPortPostProcessor()))
                 .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk())
                 .andExpect(content().contentType(MediaType.APPLICATION_JSON))
                 .andExpect(jsonPath("$.config.polling.sleep", equalTo("00:01:00")))

diff --git a/.../java/org/eclipse/hawkbit/ddi/rest/resource/RequestOnHawkbitDefaultPortPostProcessor.java b/.../java/org/eclipse/hawkbit/ddi/rest/resource/RequestOnHawkbitDefaultPortPostProcessor.java
@@ -0,0 +1,27 @@
+/**
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation
+ *
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.eclipse.hawkbit.ddi.rest.resource;
+
+import static org.eclipse.hawkbit.ddi.rest.resource.AbstractDDiApiIntegrationTest.HTTP_PORT;
+
+import org.jetbrains.annotations.NotNull;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.test.web.servlet.request.RequestPostProcessor;
+
+public class RequestOnHawkbitDefaultPortPostProcessor implements RequestPostProcessor {
+
+    @NotNull
+    @Override
+    public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) {
+        request.setRemotePort(HTTP_PORT);
+        request.setServerPort(HTTP_PORT);
+        return request;
+    }
+}