From 3aac7be11cf2d090e5f15dbef4572dbf8adba78d Mon Sep 17 00:00:00 2001
From: Kacper Magdziarz <95610011+kmagdziarz@users.noreply.github.com>
Date: Wed, 24 Aug 2022 15:59:11 +0200
Subject: [PATCH] MNT-23103 - Downloading a document replaces filename having
 space with + character (#1327)

* MNT-23103 Change encoder for content disposition filename to be consistent with RFC5987.

Add test covering filename encoding.
---
 .../web/scripts/content/ContentStreamer.java  | 23 ++++++++--
 .../alfresco/rest/api/tests/NodeApiTest.java  | 44 +++++++++++++++++++
 .../publicapi/upload/Test Download (1).txt    |  1 +
 3 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100644 remote-api/src/test/resources/publicapi/upload/Test Download (1).txt

diff --git a/remote-api/src/main/java/org/alfresco/repo/web/scripts/content/ContentStreamer.java b/remote-api/src/main/java/org/alfresco/repo/web/scripts/content/ContentStreamer.java
index 8c418ce92d..846ecb9401 100644
--- a/remote-api/src/main/java/org/alfresco/repo/web/scripts/content/ContentStreamer.java
+++ b/remote-api/src/main/java/org/alfresco/repo/web/scripts/content/ContentStreamer.java
@@ -30,6 +30,7 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.UnsupportedEncodingException;
 import java.net.SocketException;
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
@@ -59,6 +60,7 @@ import org.alfresco.service.namespace.QName;
 import org.alfresco.util.TempFileProvider;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.cxf.attachment.Rfc5987Util;
 import org.springframework.context.ResourceLoaderAware;
 import org.springframework.core.io.ResourceLoader;
 import org.springframework.extensions.webscripts.Cache;
@@ -472,7 +474,7 @@ public class ContentStreamer implements ResourceLoaderAware
 
                 if (req == null)
                 {
-                    headerValue += "; filename*=UTF-8''" + URLEncoder.encode(attachFileName, StandardCharsets.UTF_8)
+                    headerValue += "; filename*=UTF-8''" + encodeFilename(attachFileName)
                             + "; filename=\"" + filterNameForQuotedString(attachFileName) + "\"";
                 }
                 else
@@ -481,12 +483,12 @@ public class ContentStreamer implements ResourceLoaderAware
                     boolean isLegacy = (null != userAgent) && (userAgent.contains("MSIE 8") || userAgent.contains("MSIE 7"));
                     if (isLegacy)
                     {
-                        headerValue += "; filename=\"" + URLEncoder.encode(attachFileName, StandardCharsets.UTF_8);
+                        headerValue += "; filename=\"" + encodeFilename(attachFileName);
                     }
                     else
                     {
                         headerValue += "; filename=\"" + filterNameForQuotedString(attachFileName) + "\"; filename*=UTF-8''"
-                                + URLEncoder.encode(attachFileName, StandardCharsets.UTF_8);
+                                + encodeFilename(attachFileName);
                     }
                 }
             }
@@ -496,6 +498,21 @@ public class ContentStreamer implements ResourceLoaderAware
             res.setHeader("Content-Disposition", headerValue);
         }
     }
+
+    private String encodeFilename(String attachFileName)
+    {
+        try
+        {
+            return Rfc5987Util.encode(attachFileName);
+        }
+        catch (UnsupportedEncodingException e)
+        {
+            if (logger.isInfoEnabled())
+                logger.info(e.getMessage() + " Changing encoder from Rfc5987Util to java.net.URLEncoder.");
+
+            return URLEncoder.encode(attachFileName, StandardCharsets.UTF_8);
+        }
+    }
     
     protected String filterNameForQuotedString(String s)
     {
diff --git a/remote-api/src/test/java/org/alfresco/rest/api/tests/NodeApiTest.java b/remote-api/src/test/java/org/alfresco/rest/api/tests/NodeApiTest.java
index bba1bd19f7..475308c855 100644
--- a/remote-api/src/test/java/org/alfresco/rest/api/tests/NodeApiTest.java
+++ b/remote-api/src/test/java/org/alfresco/rest/api/tests/NodeApiTest.java
@@ -3721,6 +3721,50 @@ public class NodeApiTest extends AbstractSingleNetworkSiteTest
         getSingle(getNodeContentUrl(contentNodeId), null, null, headers, 304);
     }
 
+    /**
+     * Tests download of file/content name.
+     * <p>GET:</p>
+     * {@literal <host>:<port>/alfresco/api/-default-/public/alfresco/versions/1/nodes/<nodeId>/content}
+     */
+    @Test
+    public void testDownloadFileContentName() throws Exception
+    {
+        setRequestContext(user1);
+
+        //
+        // Test plain text
+        //
+
+        String fileName = "Test Download (1).txt";
+        File file = getResourceFile(fileName);
+
+        MultiPartBuilder multiPartBuilder = MultiPartBuilder.create()
+                                                            .setFileData(new FileData(fileName, file));
+        MultiPartRequest reqBody = multiPartBuilder.build();
+
+        // Upload text content
+        HttpResponse response = post(getNodeChildrenUrl(Nodes.PATH_MY), reqBody.getBody(), null, reqBody.getContentType(), 201);
+        Document document = RestApiUtil.parseRestApiEntry(response.getJsonResponse(), Document.class);
+
+        String contentNodeId = document.getId();
+
+        // Check the upload response
+        assertEquals(fileName, document.getName());
+        ContentInfo contentInfo = document.getContent();
+        assertNotNull(contentInfo);
+        assertEquals(MimetypeMap.MIMETYPE_TEXT_PLAIN, contentInfo.getMimeType());
+
+        // Download text content - by default with Content-Disposition header
+        response = getSingle(NodesEntityResource.class, contentNodeId + "/content", null, 200);
+
+        String textContent = response.getResponse();
+        assertEquals("The quick brown fox jumps over the lazy dog", textContent);
+        Map<String, String> responseHeaders = response.getHeaders();
+        assertNotNull(responseHeaders);
+        assertEquals("attachment; filename=\"Test Download (1).txt\"; filename*=UTF-8''Test%20Download%20%281%29.txt", responseHeaders.get("Content-Disposition"));
+
+    }
+
     /**
      * Tests download of file/content using backed temp file for streaming.
      * <p>
diff --git a/remote-api/src/test/resources/publicapi/upload/Test Download (1).txt b/remote-api/src/test/resources/publicapi/upload/Test Download (1).txt
new file mode 100644
index 0000000000..ff3bb63948
--- /dev/null
+++ b/remote-api/src/test/resources/publicapi/upload/Test Download (1).txt	
@@ -0,0 +1 @@
+The quick brown fox jumps over the lazy dog
\ No newline at end of file