Skip to content

Commit 7574030

Browse files
authored
s3: fix handling of NULL blob.contentType values (#1353)
* s3: fix handling of NULL blob.contentType values Ref #1351
1 parent 26cd17e commit 7574030

File tree

6 files changed

+35
-13
lines changed

6 files changed

+35
-13
lines changed

lib/external/s3.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,9 @@ const init = (config) => {
118118
// * https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html#API_GetObject_RequestSyntax
119119
const getRespHeaders = (filename, { contentType }) => ({
120120
'response-content-disposition': contentDisposition(filename),
121-
'response-content-type': contentType,
121+
// "null" is a questionable content-type, but matches current central behaviour
122+
// See: https://github.com/getodk/central-backend/pull/1352
123+
'response-content-type': contentType || 'null',
122124
});
123125

124126
async function urlForBlob(filename, blob) {
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
I should not be uploaded with a mime type.
2+
TODO what mime type should I be DOWNLOADED with?

test/e2e/s3/test-forms/1.xml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@
4949
<value>Smile</value>
5050
<value form="image">jr://images/smile.png</value>
5151
</text>
52+
<text id="geojson">
53+
<value>geojson</value>
54+
<value form="image">jr://images/cities.geojson</value>
55+
</text>
56+
<text id="nomime">
57+
<value>geojson</value>
58+
<value form="image">jr://images/some.nomime</value>
59+
</text>
5260
</translation>
5361
</itext>
5462
<instance>

test/e2e/s3/test.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,13 @@ describe('s3 support', () => {
9090

9191
// given
9292
await setup(1);
93-
await assertNewStatuses({ pending: 11 });
93+
await assertNewStatuses({ pending: 13 });
9494

9595
// when
9696
await cli('upload-pending');
9797

9898
// then
99-
await assertNewStatuses({ uploaded: 11 });
99+
await assertNewStatuses({ uploaded: 13 });
100100
// and
101101
await assertAllRedirect(actualAttachments);
102102
await assertAllDownloadsMatchOriginal(actualAttachments);
@@ -379,7 +379,10 @@ describe('s3 support', () => {
379379

380380
const filepath = `${attDir}/${name}`;
381381

382-
const expectedContentType = mimetypeFor(name);
382+
// "null" is a questionable content-type, but matches current central behaviour
383+
// See: https://github.com/getodk/central-backend/pull/1352
384+
const expectedContentType = mimetypeFor(name) ?? 'null';
385+
383386
const actualContentType = res.headers.get('content-type');
384387
should.equal(actualContentType, expectedContentType);
385388

test/e2e/util/api.js

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,11 @@ async function apiClient(suiteName, { serverUrl, userEmail, userPassword, logPat
4646
});
4747
} else {
4848
const { body, mimeType } = opts;
49-
return apiPost(path, body, { 'Content-Type':mimeType });
49+
50+
const headers = {};
51+
if(mimeType) headers['Content-Type'] = mimeType;
52+
53+
return apiPost(path, body, headers);
5054
}
5155
}
5256

@@ -119,14 +123,16 @@ function mimetypeFor(f) {
119123
// For more, see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types
120124
const extension = extname(f);
121125
switch(extension) {
122-
case '.bin' : return 'application/octet-stream';
123-
case '.jpg' : return 'image/jpeg';
124-
case '.png' : return 'image/png';
125-
case '.svg' : return 'image/svg+xml';
126-
case '.txt' : return 'text/plain';
127-
case '.xls' : return 'application/vnd.ms-excel';
128-
case '.xlsx': return 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet';
129-
case '.xml' : return 'application/xml';
126+
case '.bin' : return 'application/octet-stream';
127+
case '.geojson': return 'application/geo+json';
128+
case '.jpg' : return 'image/jpeg';
129+
case '.nomime' : return null; // used for testing user agents which do not set a mime type for some file extensions
130+
case '.png' : return 'image/png';
131+
case '.svg' : return 'image/svg+xml';
132+
case '.txt' : return 'text/plain';
133+
case '.xls' : return 'application/vnd.ms-excel';
134+
case '.xlsx' : return 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet';
135+
case '.xml' : return 'application/xml';
130136
default: throw new Error(`Unsure what mime type to use for: ${f}`);
131137
}
132138
}

0 commit comments

Comments
 (0)