fixes 433: convert latin1 to utf for filenames return by multer (#908)

getodk · May 30, 2023 · 3095cb0 · 3095cb0
1 parent 0661a7b
commit 3095cb0
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 4 deletions.
diff --git a/lib/resources/submissions.js b/lib/resources/submissions.js
@@ -25,6 +25,8 @@ const { resolve } = require('../util/promise');
 const { isBlank, noargs } = require('../util/util');
 const { zipStreamFromParts } = require('../util/zip');
 
+const XML_SUBMISSION_FILE = 'xml_submission_file';
+
 // multipart things:
 const multipart = multer({ storage: multer.memoryStorage() });
 
@@ -33,11 +35,11 @@ const bodyParser = require('body-parser');
 const formParser = bodyParser.urlencoded({ extended: false });
 
 // util funcs for openRosaSubmission below, mostly just moving baked logic/data off for perf.
-const missingXmlProblem = Problem.user.missingMultipartField({ field: 'xml_submission_file' });
+const missingXmlProblem = Problem.user.missingMultipartField({ field: XML_SUBMISSION_FILE });
 const findMultipart = (files) => {
   if (files == null) throw missingXmlProblem;
   for (let i = 0; i < files.length; i += 1)
-    if (files[i].fieldname === 'xml_submission_file')
+    if (files[i].fieldname === XML_SUBMISSION_FILE)
       return files[i];
   throw missingXmlProblem;
 };
@@ -64,8 +66,22 @@ module.exports = (service, endpoint) => {
         .then(getOrNotFound)
         .then(always({ code: 204, body: '' }))));
 
+    // Temporary solution to https://github.com/expressjs/multer/issues/1104
+    // Multer uses latin1 encoding for filename and fieldname
+    const multerUtf = (request, _, next) => {
+      request.files = request.files?.map(f => {
+        if (f.fieldname === XML_SUBMISSION_FILE) return f;
+        return {
+          ...f,
+          originalname: Buffer.from(f.originalname, 'latin1').toString('utf8'),
+          fieldname: Buffer.from(f.fieldname, 'latin1').toString('utf8')
+        };
+      });
+      next();
+    };
+
     // Nonstandard REST; OpenRosa-specific API.
-    service.post(path, multipart.any(), endpoint.openRosa(({ Forms, Submissions, SubmissionAttachments }, { params, files, auth, query, headers }) =>
+    service.post(path, multipart.any(), multerUtf, endpoint.openRosa(({ Forms, Submissions, SubmissionAttachments }, { params, files, auth, query, headers }) =>
       Submission.fromXml(findMultipart(files).buffer)
         .then((partial) => getForm(auth, params, partial.xmlFormId, Forms, partial.def.version)
           .catch(forceAuthFailed)

diff --git a/test/data/xml.js b/test/data/xml.js
@@ -543,7 +543,8 @@ module.exports = {
     binaryType: {
       one: instance('binaryType', 'bone', '<file1>my_file1.mp4</file1>'),
       two: instance('binaryType', 'btwo', '<file2>here_is_file2.jpg</file2>'),
-      both: instance('binaryType', 'both', '<file1>my_file1.mp4</file1><file2>here_is_file2.jpg</file2>')
+      both: instance('binaryType', 'both', '<file1>my_file1.mp4</file1><file2>here_is_file2.jpg</file2>'),
+      unicode: instance('binaryType', 'both', '<file1>fîlé2</file1><file2>f😂le3صادق</file2>'),
     },
     encrypted: {
       // TODO: the jpg binary associated with this sample blob is >3MB. will replace

diff --git a/test/integration/api/submissions.js b/test/integration/api/submissions.js
@@ -223,6 +223,44 @@ describe('api: /submission', () => {
                 ]);
               }))))));
 
+    it('should save attachments with unicode / non-english char', testService(async (service) => {
+      const asAlice = await service.login('alice');
+
+      await asAlice.post('/v1/projects/1/forms?publish=true')
+        .set('Content-Type', 'application/xml')
+        .send(testData.forms.binaryType)
+        .expect(200);
+
+      await asAlice.post('/v1/projects/1/submission')
+        .set('X-OpenRosa-Version', '1.0')
+        .attach('xml_submission_file', Buffer.from(testData.instances.binaryType.unicode), { filename: 'data.xml' })
+        .attach('fîlé2', Buffer.from('this is test file one'), { filename: 'fîlé2.bin' })
+        .attach('f😂le3صادق', Buffer.from('this is test file two'), { filename: 'f😂le3صادق' })
+        .expect(201);
+
+      await asAlice.get('/v1/projects/1/forms/binaryType/submissions/both/attachments')
+        .expect(200)
+        .then(({ body }) => {
+          body.should.eql([
+            { name: 'fîlé2', exists: true },
+            { name: 'f😂le3صادق', exists: true }
+          ]);
+        });
+
+      await asAlice.get(`/v1/projects/1/forms/binaryType/submissions/both/attachments/${encodeURI('fîlé2')}`)
+        .expect(200)
+        .then(({ body }) => {
+          body.toString('utf8').should.be.eql('this is test file one');
+        });
+
+      await asAlice.get(`/v1/projects/1/forms/binaryType/submissions/both/attachments/${encodeURI('f😂le3صادق')}`)
+        .expect(200)
+        .then(({ body }) => {
+          body.toString('utf8').should.be.eql('this is test file two');
+        });
+
+    }));
+
     it('should not fail given identical attachments', testService((service) =>
       service.login('alice', (asAlice) =>
         asAlice.post('/v1/projects/1/forms?publish=true')