fix: Sanitize percent characters in resource URLs (#24031) (CP: 25.1) (#24169)

This PR cherry-picks changes from the original PR #24031 to branch 25.1.
---
#### Original PR description
> Jetty 12 rejects URLs containing %25 (percent-encoded percent) as
ambiguous URI path encoding, causing downloads to fail with HTTP 400
when filenames contain "%" characters.
> 
> Add UrlUtil.sanitizeForUrl() that replaces "%" with "_" in the URL
path segment. The actual download filename from Content-Disposition is
unaffected since each resource has a unique ID for lookup.
> 
> Fixes #22677
>

Co-authored-by: Tomi Virtanen <tltv@vaadin.com>
Co-authored-by: Artur Signell <artur@vaadin.com>
Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>

Author: 4 people

flow-server/src/main/java/com/vaadin/flow/server/communication/StreamRequestHandler.java

@@ -244,9 +244,30 @@ private PathData parsePath(String pathInfo) {
      * @return generated URI string
      */
     public static String generateURI(String name, String id) {
+        // Replace '%' with '_' to avoid ambiguous percent-encoding (%25)
+        // rejected by Jetty 12. The actual download filename is preserved
+        // in the Content-Disposition header, so this only affects the URL
+        // path used for resource lookup.
+        String safeName = sanitizeNameForUrl(name);
         return DYN_RES_PREFIX + UI.getCurrentOrThrow().getUIId()
                 + PATH_SEPARATOR + id + PATH_SEPARATOR
-                + UrlUtil.encodeURIComponent(name);
+                + UrlUtil.encodeURIComponent(safeName);
+    }
+
+    /**
+     * Replaces characters that would produce ambiguous percent-encodings in URL
+     * path segments.
+     *
+     * @param name
+     *            the resource name to sanitize
+     * @return the sanitized name, or the original value if {@code null} or
+     *         empty
+     */
+    static String sanitizeNameForUrl(String name) {
+        if (name == null || name.isEmpty()) {
+            return name;
+        }
+        return name.replace("%", "_");
     }
 
     private static Optional<URI> getPathUri(String path) {

flow-server/src/test/java/com/vaadin/flow/server/communication/StreamRequestHandlerTest.java

@@ -50,6 +50,8 @@
 
 import static com.vaadin.flow.server.communication.StreamRequestHandler.DYN_RES_PREFIX;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
 @NotThreadSafe
 class StreamRequestHandlerTest {
@@ -155,6 +157,18 @@ void streamResourceNameContainsPlusAndSpaces_resourceWriter_resourceIsStreamed()
                 "readme + mine.md");
     }
 
+    @Test
+    public void streamResourceNameContainsPercent_streamFactory_resourceIsStreamed()
+            throws IOException {
+        testStreamResourceInputStreamFactory("percent in name", "file%.txt");
+    }
+
+    @Test
+    public void streamResourceNameContainsPercent_resourceWriter_resourceIsStreamed()
+            throws IOException {
+        testStreamResourceStreamResourceWriter("percent in name", "file%.txt");
+    }
+
     @Test
     void stateNodeStates_handlerMustNotReplyWhenNodeDisabled()
             throws IOException {
@@ -247,9 +261,10 @@ private VaadinResponse stateNodeStatesTestInternal(
         ServletOutputStream outputStream = Mockito
                 .mock(ServletOutputStream.class);
         Mockito.when(response.getOutputStream()).thenReturn(outputStream);
-        Mockito.when(request.getPathInfo())
-                .thenReturn(String.format("/%s%s/%s/%s", DYN_RES_PREFIX,
-                        ui.getId().orElse("-1"), res.getId(), res.getName()));
+        Mockito.when(request.getPathInfo()).thenReturn(String.format(
+                "/%s%s/%s/%s", DYN_RES_PREFIX, ui.getId().orElse("-1"),
+                res.getId(),
+                StreamRequestHandler.sanitizeNameForUrl(res.getName())));
 
         handler.handleRequest(session, request, response);
 
@@ -268,9 +283,10 @@ private void testStreamResourceInputStreamFactory(String testString,
         ServletOutputStream outputStream = Mockito
                 .mock(ServletOutputStream.class);
         Mockito.when(response.getOutputStream()).thenReturn(outputStream);
-        Mockito.when(request.getPathInfo())
-                .thenReturn(String.format("/%s%s/%s/%s", DYN_RES_PREFIX,
-                        ui.getId().orElse("-1"), res.getId(), res.getName()));
+        Mockito.when(request.getPathInfo()).thenReturn(String.format(
+                "/%s%s/%s/%s", DYN_RES_PREFIX, ui.getId().orElse("-1"),
+                res.getId(),
+                StreamRequestHandler.sanitizeNameForUrl(res.getName())));
 
         handler.handleRequest(session, request, response);
 
@@ -302,9 +318,10 @@ private void testStreamResourceStreamResourceWriter(String testString,
         ServletOutputStream outputStream = Mockito
                 .mock(ServletOutputStream.class);
         Mockito.when(response.getOutputStream()).thenReturn(outputStream);
-        Mockito.when(request.getPathInfo())
-                .thenReturn(String.format("/%s%s/%s/%s", DYN_RES_PREFIX,
-                        ui.getId().orElse("-1"), res.getId(), res.getName()));
+        Mockito.when(request.getPathInfo()).thenReturn(String.format(
+                "/%s%s/%s/%s", DYN_RES_PREFIX, ui.getId().orElse("-1"),
+                res.getId(),
+                StreamRequestHandler.sanitizeNameForUrl(res.getName())));
 
         handler.handleRequest(session, request, response);
 
@@ -319,6 +336,22 @@ private void testStreamResourceStreamResourceWriter(String testString,
         Mockito.verify(response).setContentType("application/octet-stream");
     }
 
+    @Test
+    public void sanitizeNameForUrl_percentIsReplaced() {
+        assertEquals("file_.txt",
+                StreamRequestHandler.sanitizeNameForUrl("file%.txt"));
+    }
+
+    @Test
+    public void sanitizeNameForUrl_nullReturnsNull() {
+        assertNull(StreamRequestHandler.sanitizeNameForUrl(null));
+    }
+
+    @Test
+    public void sanitizeNameForUrl_emptyReturnsEmpty() {
+        assertEquals("", StreamRequestHandler.sanitizeNameForUrl(""));
+    }
+
     private static final class TestElementHandlerBuilder {
         private final TestElementHandlerProperties elementHandlerProperties;
         private final TestStateNodeProperties stateNodeProperties;

flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/DownloadHandlerView.java

@@ -130,10 +130,27 @@ public String getUrlPostfix() {
         inputStreamCallbackError
                 .setId("download-handler-input-stream-callback-error");
 
+        DownloadHandler percentDownloadHandler = new DownloadHandler() {
+            @Override
+            public void handleDownloadRequest(DownloadEvent event) {
+                event.getWriter().print("foo");
+            }
+
+            @Override
+            public String getUrlPostfix() {
+                return "file%.jpg";
+            }
+        };
+
+        Anchor percentDownload = new Anchor("", "Percent DownloadHandler");
+        percentDownload.setHref(percentDownloadHandler);
+        percentDownload.setId("download-handler-percent");
+
         add(handlerDownload, fileDownload, fileDownloadUnicodeName,
                 fileDownloadUnicodeNameWithQuote, classDownload,
                 servletDownload, inputStreamDownload, inputStreamErrorDownload,
-                inputStreamExceptionDownload, inputStreamCallbackError);
+                inputStreamExceptionDownload, inputStreamCallbackError,
+                percentDownload);
 
         NativeButton reattach = new NativeButton("Remove and add back",
                 event -> {

flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/DownloadHandlerIT.java

@@ -222,6 +222,13 @@ public void getDynamicDownloadHandlerFailingInputStreamCallback_errorIsReceived(
                 .withTextContaining("java.io.IOException: Callback").exists());
     }
 
+    @Test
+    public void getDynamicDownloadHandlerPercentResource() throws IOException {
+        open();
+
+        assertDownloadedContent("download-handler-percent", "file_.jpg");
+    }
+
     @Test
     public void detach_attachALink_getDynamicVaadinResource()
             throws IOException {

flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/StreamResourceIT.java

@@ -26,7 +26,6 @@
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.io.IOUtils;
 import org.junit.Assert;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.openqa.selenium.By;
 import org.openqa.selenium.WebElement;
@@ -54,14 +53,11 @@ public void getDynamicVaadinPlusResource() throws IOException {
         assertDownloadedContent("plus-link", "file%2B.jpg");
     }
 
-    // Ignored due to
-    // https://github.com/jetty/jetty.project/issues/9444#issuecomment-1677068428
     @Test
-    @Ignore
     public void getDynamicVaadinPercentResource() throws IOException {
         open();
 
-        assertDownloadedContent("percent-link", "file%25.jpg");
+        assertDownloadedContent("percent-link", "file_.jpg");
     }
 
     @Test