blobs: derive missing contentType from file extension

Ref getodk#1351

lib/model/query/submission-attachments.js

@@ -20,7 +20,7 @@ const { insertMany, QueryOptions } = require('../../util/db');
 const { resolve } = require('../../util/promise');
 const { isBlank, construct } = require('../../util/util');
 const { traverseXml, findAll, root, node, text } = require('../../util/xml');
-const { streamBlobs } = require('../../util/blob');
+const { defaultMimetypeFor, streamBlobs } = require('../../util/blob');
 
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -29,7 +29,7 @@ const { streamBlobs } = require('../../util/blob');
 const _makeAttachment = (ensure, submissionDefId, name, file = null, deprecated = null, index = null, isClientAudit = null) => {
   const data = { submissionDefId, name, index, isClientAudit, blobId: deprecated };
   return (file == null) ? resolve(new Submission.Attachment(data))
-    : ensure(Blob.fromBuffer(file.buffer, file.mimetype)).then((blobId) => {
+    : ensure(Blob.fromBuffer(file.buffer, file.mimetype || defaultMimetypeFor(name))).then((blobId) => {
       data.blobId = blobId;
       return new Submission.Attachment(data);
     });
@@ -106,7 +106,7 @@ const upsert = (def, files) => ({ Blobs, SubmissionAttachments }) =>
       const lookup = new Set(expecteds.map((att) => att.name));
       const present = files.filter((file) => lookup.has(file.fieldname));
       return Promise.all(present
-        .map((file) => Blobs.ensure(Blob.fromBuffer(file.buffer, file.mimetype))
+        .map((file) => Blobs.ensure(Blob.fromBuffer(file.buffer, file.mimetype || defaultMimetypeFor(file.fieldname)))
           .then((blobId) => SubmissionAttachments.attach(def, file.fieldname, blobId))));
     });
 

lib/resources/forms.js

@@ -13,7 +13,7 @@ const { Blob, Form } = require('../model/frames');
 const { ensureDef } = require('../model/frame');
 const { QueryOptions } = require('../util/db');
 const { isTrue, xml, contentDisposition, withEtag } = require('../util/http');
-const { blobResponse } = require('../util/blob');
+const { blobResponse, defaultMimetypeFor } = require('../util/blob');
 const Problem = require('../util/problem');
 const { sanitizeFieldsForOdata, setVersion } = require('../data/schema');
 const { getOrNotFound, reject, resolve, rejectIf } = require('../util/promise');
@@ -380,7 +380,7 @@ module.exports = (service, endpoint) => {
       .then(getOrNotFound)
       .then((form) => auth.canOrReject('form.update', form))
       .then((form) => Promise.all([
-        Blob.fromStream(request, headers['content-type']).then((blob) => Blobs.ensure(blob)),
+        Blob.fromStream(request, headers['content-type'] || defaultMimetypeFor(params.name)).then((blob) => Blobs.ensure(blob)),
         FormAttachments.getByFormDefIdAndName(form.draftDefId, params.name).then(getOrNotFound)
       ])
         .then(([ blobId, attachment ]) => FormAttachments.update(form, attachment, blobId, null))

lib/resources/submissions.js

@@ -16,7 +16,7 @@ const { createdMessage } = require('../formats/openrosa');
 const { getOrNotFound, getOrReject, rejectIf, reject } = require('../util/promise');
 const { QueryOptions } = require('../util/db');
 const { success, xml, isFalse, contentDisposition, redirect, url } = require('../util/http');
-const { blobResponse } = require('../util/blob');
+const { blobResponse, defaultMimetypeFor } = require('../util/blob');
 const Problem = require('../util/problem');
 const { streamBriefcaseCsvs } = require('../data/briefcase');
 const { streamAttachments } = require('../data/attachments');
@@ -446,7 +446,7 @@ module.exports = (service, endpoint) => {
               .then((def) => SubmissionAttachments.getBySubmissionDefIdAndName(def.id, params.name) // just for audit logging
                 .then(getOrNotFound)
                 .then((oldAttachment) => [ form, def, oldAttachment ]))),
-          Blob.fromStream(request, headers['content-type']).then(Blobs.ensure)
+          Blob.fromStream(request, headers['content-type'] || defaultMimetypeFor(params.name)).then(Blobs.ensure)
         ])
           .then(([ [ form, def, oldAttachment ], blobId ]) => Promise.all([
             SubmissionAttachments.attach(def, params.name, blobId),

lib/util/blob.js

@@ -7,6 +7,7 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
+const { extname } = require('node:path');
 const { Transform } = require('stream');
 const { PartialPipe } = require('./stream');
 const { contentDisposition, redirect, withEtag } = require('./http');
@@ -67,4 +68,13 @@ async function blobResponse(s3, filename, blob) {
   }
 }
 
-module.exports = { blobContent, blobResponse, streamBlobs, streamEncBlobs };
+function defaultMimetypeFor(filename) {
+  if (!filename) return null;
+  switch (extname(filename)) {
+    case '.csv':     return 'text/csv'; // eslint-disable-line no-multi-spaces
+    case '.geojson': return 'application/geo+json';
+    default:         return null; // eslint-disable-line no-multi-spaces
+  }
+}
+
+module.exports = { blobContent, blobResponse, defaultMimetypeFor, streamBlobs, streamEncBlobs };

test/data/xml.js

@@ -637,6 +637,7 @@ module.exports = {
       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>'),
       unicode: instance('binaryType', 'both', '<file1>fîlé2</file1><file2>f😂le3صادق</file2>'),
+      withFile: (filename) => instance('binaryType', 'with-file', `<file1>${filename}</file1>`),
     },
     encrypted: {
       // TODO: the jpg binary associated with this sample blob is >3MB. will replace

test/integration/api/submissions.js

@@ -4421,6 +4421,78 @@ one,h,/data/h,2000-01-01T00:06,2000-01-01T00:07,-5,-6,,ee,ff
                   text.toString().should.equal('testvideo'); // use 'text' instead of 'body' to avoid supertest response parsing
                 })))))));
 
+    [
+      // express ALWAYS adds "charset=..." suffix to text-based Content-Type response headers
+      // See: https://github.com/expressjs/express/issues/2654
+      [ 'CSV',     'myfile.csv',     'text/csv; charset=utf-8', 'a,b,c' ], // eslint-disable-line no-multi-spaces
+      [ 'GeoJSON', 'myfile.geojson', 'application/geo+json',    '{}' ], // eslint-disable-line no-multi-spaces
+    ].forEach(([ humanType, filename, officialContentType, fileContents ]) => {
+      describe(`special handling for ${humanType}`, () => {
+        it('should attach the given file and serve it with supplied mime type', testService((service) =>
+          service.login('alice', (asAlice) =>
+            asAlice.post('/v1/projects/1/forms?publish=true')
+              .set('Content-Type', 'application/xml')
+              .send(testData.forms.binaryType)
+              .expect(200)
+              .then(() => asAlice.post('/v1/projects/1/forms/binaryType/submissions')
+                .send(testData.instances.binaryType.withFile(filename))
+                .set('Content-Type', 'text/xml')
+                .expect(200)
+                .then(() => asAlice.post(`/v1/projects/1/forms/binaryType/submissions/with-file/attachments/${filename}`)
+                  .set('Content-Type', 'application/x-abiword')
+                  .send(fileContents)
+                  .expect(200)
+                  .then(() => asAlice.get(`/v1/projects/1/forms/binaryType/submissions/with-file/attachments/${filename}`)
+                    .expect(200)
+                    .then(({ headers, text }) => {
+                      headers['content-type'].should.equal('application/x-abiword');
+                      text.toString().should.equal(fileContents); // use 'text' instead of 'body' to avoid supertest response parsing
+                    })))))));
+
+        it(`should attach a given ${humanType} file with empty Content-Type and serve it with correct mime type`, testService((service) =>
+          service.login('alice', (asAlice) =>
+            asAlice.post('/v1/projects/1/forms?publish=true')
+              .set('Content-Type', 'application/xml')
+              .send(testData.forms.binaryType)
+              .expect(200)
+              .then(() => asAlice.post('/v1/projects/1/forms/binaryType/submissions')
+                .send(testData.instances.binaryType.withFile(filename))
+                .set('Content-Type', 'text/xml')
+                .expect(200)
+                .then(() => asAlice.post(`/v1/projects/1/forms/binaryType/submissions/with-file/attachments/${filename}`)
+                  .send(fileContents)
+                  .set('Content-Type', '') // N.B. must be called _after_ send()
+                  .expect(200)
+                  .then(() => asAlice.get(`/v1/projects/1/forms/binaryType/submissions/with-file/attachments/${filename}`)
+                    .expect(200)
+                    .then(({ headers, text }) => {
+                      headers['content-type'].should.equal(officialContentType);
+                      text.toString().should.equal(fileContents); // use 'text' instead of 'body' to avoid supertest response parsing
+                    })))))));
+
+        it(`should attach a given ${humanType} file with missing Content-Type and serve it with correct mime type`, testService((service) =>
+          service.login('alice', (asAlice) =>
+            asAlice.post('/v1/projects/1/forms?publish=true')
+              .set('Content-Type', 'application/xml')
+              .send(testData.forms.binaryType)
+              .expect(200)
+              .then(() => asAlice.post('/v1/projects/1/forms/binaryType/submissions')
+                .send(testData.instances.binaryType.withFile(filename))
+                .set('Content-Type', 'text/xml')
+                .expect(200)
+                .then(() => asAlice.post(`/v1/projects/1/forms/binaryType/submissions/with-file/attachments/${filename}`)
+                  .send(fileContents)
+                  .unset('Content-Type') // N.B. must be called _after_ send()
+                  .expect(200)
+                  .then(() => asAlice.get(`/v1/projects/1/forms/binaryType/submissions/with-file/attachments/${filename}`)
+                    .expect(200)
+                    .then(({ headers, text }) => {
+                      headers['content-type'].should.equal(officialContentType);
+                      text.toString().should.equal(fileContents); // use 'text' instead of 'body' to avoid supertest response parsing
+                    })))))));
+      });
+    });
+
     it('should log an audit entry about initial attachment', testService((service, { Audits, Forms, Submissions, SubmissionAttachments }) =>
       service.login('alice', (asAlice) =>
         asAlice.post('/v1/projects/1/forms?publish=true')