viritin / flow-viritin

Viritin inspired project for Vaadin Flow
Other
39 stars 15 forks source link

uploadfilehandler propagates the generated uuid of the file instead of the real file name #54

Closed jorgheymans closed 2 months ago

jorgheymans commented 2 months ago

When uploading a file through UploadFileHandler, it propagates the generated UUID (ufhid in the js) as filename instead of the real file name. Given that the javadoc for FileHandler says @param fileName the name of the file in users device I believe the current behaviour is wrong and we can change it without breaking compat. See below how i fixed it locally, using a more "formal" content disposition parser at the expense of an extra dependency.

diff --git a/pom.xml b/pom.xml
index 420393e..67c939e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -103,6 +103,13 @@
             <version>5.0.0</version>
             <scope>provided</scope>
         </dependency>
+
+        <dependency>
+            <groupId>jakarta.mail</groupId>
+            <artifactId>jakarta.mail-api</artifactId>
+            <version>2.1.3</version>
+        </dependency>
+
         <dependency>
             <groupId>commons-beanutils</groupId>
             <artifactId>commons-beanutils</artifactId>
diff --git a/src/main/java/org/vaadin/firitin/components/upload/UploadFileHandler.java b/src/main/java/org/vaadin/firitin/components/upload/UploadFileHandler.java
index db92771..102a2fc 100644
--- a/src/main/java/org/vaadin/firitin/components/upload/UploadFileHandler.java
+++ b/src/main/java/org/vaadin/firitin/components/upload/UploadFileHandler.java
@@ -44,6 +44,8 @@ import org.vaadin.firitin.fluency.ui.FluentHasEnabled;
 import org.vaadin.firitin.fluency.ui.FluentHasSize;
 import org.vaadin.firitin.fluency.ui.FluentHasStyle;

+import jakarta.mail.internet.ContentDisposition;
+import jakarta.mail.internet.ParseException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -382,9 +384,17 @@ public class UploadFileHandler extends Component implements FluentComponent<Uplo
                 // Vaadin's StreamReceiver & friends has this odd
                 // inversion of streams, thus handle here
                 // TODO figure out if content type or name needs some sanitation
+                ContentDisposition contentDisposition = null;
+                try {
+                    contentDisposition = new ContentDisposition(cd);
+                } catch (ParseException e) {
+                    // TODO throw up or act
+                }
+                String name = null;
+                if (contentDisposition != null) {
+                   name = contentDisposition.getParameter("filename");
+                }
                 String contentType = request.getHeader("Content-Type");
-                String name = cd.split(";")[1].split("=")[1].substring(1);
-                name = name.substring(0, name.indexOf("\""));
                 name = URLDecoder.decode(name, "UTF-8");
                 Command cb = fileHandler.handleFile(request.getInputStream(), new FileDetails(name, contentType));
                 if (cb != null) {

On my machine the Content disposition header looks like this, so it's clear we have to use the filename parameter and not the name parameter.

Content-Disposition attachment;name="87761d51-4200-4c3e-8ce1-fcac9f908586"; filename="Request_9_19042024154545.xlsx"

mstahv commented 2 months ago

That indeed seems to be the case. Probably a regression since the last largish changes I made for better Spring Security support. I only consume the content so didn't notice, good catch!

Fixed now in git, building 2.8.14 soon.