Skip to content

Commit b71f3e5

Browse files
authored
Issue 50628: RFC 6266 compliant parsing of "Content-Disposition" (#182)
1 parent 65381ef commit b71f3e5

10 files changed

+227
-155
lines changed

CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
### TBD
22
- Deprecate and strongly discourage use of bitmask-based permission constants and functions
33

4+
### 1.35.1 - 2024-09-12
5+
- Issue 50628: Switch our "Content-Disposition" header parsing to utilize the `@tinyhttp/content-disposition` package.
6+
47
### 1.35.0 - 2024-07-24
58
- Package updates
69

jest.babel.config.js

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module.exports = {
2+
env: {
3+
test: {
4+
plugins: ['@babel/plugin-transform-modules-commonjs'],
5+
},
6+
},
7+
};

jest.config.js

+20-8
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,38 @@
1+
// Run all tests with timezone set to UTC
2+
// https://stackoverflow.com/a/56482581
3+
process.env.TZ = 'UTC';
4+
5+
// These are ES modules that we utilize that need to be transformed
6+
// by babel when being imported during a jest test.
7+
const esModules = ['@tinyhttp/content-disposition', 'sinon'].join('|');
8+
19
module.exports = {
210
globals: {
311
LABKEY: {
412
contextPath: '',
513
defaultHeaders: {
6-
'X-LABKEY-CSRF': 'TEST_CSRF_TOKEN'
7-
}
8-
}
14+
'X-LABKEY-CSRF': 'TEST_CSRF_TOKEN',
15+
},
16+
},
917
},
1018
moduleFileExtensions: ['ts', 'js'],
11-
moduleNameMapper: {
12-
'^sinon$': require.resolve('sinon'),
13-
},
1419
testEnvironment: 'jsdom',
1520
testResultsProcessor: 'jest-teamcity-reporter',
1621
testRegex: '(\\.(spec))\\.(ts)$',
1722
transform: {
23+
'\\.js$': [
24+
'babel-jest',
25+
{
26+
configFile: './jest.babel.config.js',
27+
},
28+
],
1829
'^.+\\.ts$': [
1930
'ts-jest',
2031
{
2132
// This increases test perf by a considerable margin
2233
isolatedModules: true,
23-
}
34+
},
2435
],
2536
},
26-
};
37+
transformIgnorePatterns: [`/node_modules/(?!${esModules})`],
38+
};

package-lock.json

+34-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@labkey/api",
3-
"version": "1.35.0",
3+
"version": "1.35.1",
44
"description": "JavaScript client API for LabKey Server",
55
"scripts": {
66
"build": "npm run build:dist && npm run build:docs",
@@ -36,7 +36,9 @@
3636
},
3737
"devDependencies": {
3838
"@babel/core": "7.24.9",
39+
"@babel/plugin-transform-modules-commonjs": "7.24.8",
3940
"@labkey/eslint-config-base": "0.0.15",
41+
"@tinyhttp/content-disposition": "2.2.1",
4042
"@types/jest": "29.5.12",
4143
"@types/sinon": "17.0.3",
4244
"cross-env": "7.0.3",

src/labkey/Ajax.spec.ts

+114-94
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import sinon from 'sinon';
1818
import * as Ajax from './Ajax';
1919
import { getFilenameFromContentDisposition } from './Ajax';
2020

21-
function mockXHR() {
21+
function mockXHR(): void {
2222
let xhr: sinon.SinonFakeXMLHttpRequestStatic;
2323

2424
afterEach(() => {
@@ -41,109 +41,129 @@ function request(options: Ajax.RequestOptions): FakeXMLHttpRequest {
4141
return Ajax.request(options) as FakeXMLHttpRequest;
4242
}
4343

44-
describe('request', () => {
45-
mockXHR();
46-
47-
it('should require configuration', () => {
48-
expect(() => {
49-
(Ajax.request as any)();
50-
}).toThrow('a URL is required to make a request');
51-
expect(() => {
52-
(Ajax.request as any)({});
53-
}).toThrow('a URL is required to make a request');
54-
});
55-
it('should make request with only url', () => {
56-
expect(request({ url: '/users' }).url).toEqual('/users');
44+
describe('Ajax', () => {
45+
describe('request', () => {
46+
mockXHR();
47+
48+
it('should require configuration', () => {
49+
expect(() => {
50+
(Ajax.request as any)();
51+
}).toThrow('a URL is required to make a request');
52+
expect(() => {
53+
(Ajax.request as any)({});
54+
}).toThrow('a URL is required to make a request');
55+
});
56+
it('should make request with only url', () => {
57+
expect(request({ url: '/users' }).url).toEqual('/users');
58+
});
5759
});
58-
});
59-
60-
describe('request headers', () => {
61-
mockXHR();
6260

63-
const testCSRF = 'TEST_CSRF_TOKEN';
64-
const contentTypeForm = 'application/x-www-form-urlencoded;charset=utf-8';
61+
describe('request headers', () => {
62+
mockXHR();
6563

66-
it('should apply DEFAULT_HEADERS', () => {
67-
const requestHeaders = request({ url: '/projects' }).requestHeaders;
68-
69-
expect(requestHeaders['Content-Type']).toEqual(contentTypeForm);
70-
expect(requestHeaders['X-Requested-With']).toEqual('XMLHttpRequest');
71-
expect(requestHeaders['X-LABKEY-CSRF']).toEqual(testCSRF);
72-
});
73-
it('should apply custom headers', () => {
74-
const requestHeaders = request({
75-
url: '/projects',
76-
headers: {
77-
foo: 'bar',
78-
},
79-
}).requestHeaders;
80-
81-
expect(requestHeaders['foo']).toEqual('bar');
82-
expect(requestHeaders['X-LABKEY-CSRF']).toEqual(testCSRF); // it shouldn't lose other headers
83-
});
84-
});
64+
const testCSRF = 'TEST_CSRF_TOKEN';
65+
const contentTypeForm = 'application/x-www-form-urlencoded;charset=utf-8';
8566

86-
describe('request method', () => {
87-
mockXHR();
67+
it('should apply DEFAULT_HEADERS', () => {
68+
const requestHeaders = request({ url: '/projects' }).requestHeaders;
8869

89-
it('should default to GET', () => {
90-
expect(request({ url: '/users' }).method).toEqual('GET');
91-
});
92-
it('should default to POST with data', () => {
93-
expect(
94-
request({
95-
url: '/users',
96-
jsonData: {
97-
userId: 123,
98-
},
99-
}).method
100-
).toEqual('POST');
101-
});
102-
it('should accept GET with data', () => {
103-
expect(
104-
request({
105-
url: '/users',
106-
method: 'GET',
107-
jsonData: {
108-
userId: 123,
70+
expect(requestHeaders['Content-Type']).toEqual(contentTypeForm);
71+
expect(requestHeaders['X-Requested-With']).toEqual('XMLHttpRequest');
72+
expect(requestHeaders['X-LABKEY-CSRF']).toEqual(testCSRF);
73+
});
74+
it('should apply custom headers', () => {
75+
const requestHeaders = request({
76+
url: '/projects',
77+
headers: {
78+
foo: 'bar',
10979
},
110-
}).method
111-
).toEqual('GET');
112-
});
113-
it('should accept any method', () => {
114-
expect(request({ url: '/users', method: 'DELETE' }).method).toEqual('DELETE');
115-
expect(request({ url: '/users', method: 'PUT' }).method).toEqual('PUT');
116-
expect(request({ url: '/users', method: 'BEEP' }).method).toEqual('BEEP');
117-
});
118-
});
119-
120-
describe('getFilenameFromContentDisposition', () => {
121-
it('should not find a file if there is no attachment prefix', () => {
122-
expect(getFilenameFromContentDisposition('other: filename=data.tsv')).toBeUndefined();
123-
expect(getFilenameFromContentDisposition('other: filename=attachment.tsv')).toBeUndefined();
124-
});
80+
}).requestHeaders;
12581

126-
it('should find a file with the filename= prefix', () => {
127-
expect(getFilenameFromContentDisposition('attachment; filename=data.xlsx')).toBe('data.xlsx');
128-
expect(getFilenameFromContentDisposition('attachment; filename=d%20ata.xlsx')).toBe('d ata.xlsx');
82+
expect(requestHeaders['foo']).toEqual('bar');
83+
expect(requestHeaders['X-LABKEY-CSRF']).toEqual(testCSRF); // it shouldn't lose other headers
84+
});
12985
});
13086

131-
it('should find a file with the filename*= prefix', () => {
132-
expect(getFilenameFromContentDisposition('attachment; filename*=data.xlsx')).toBe('data.xlsx');
133-
expect(getFilenameFromContentDisposition('attachment; filename*=d%20ata.xlsx')).toBe('d ata.xlsx');
134-
expect(getFilenameFromContentDisposition("attachment; filename*=UTF-8''data.xlsx")).toBe('data.xlsx');
135-
expect(getFilenameFromContentDisposition("attachment; filename*=UTF-8''d%20ata.xlsx")).toBe('d ata.xlsx');
87+
describe('request method', () => {
88+
mockXHR();
89+
90+
it('should default to GET', () => {
91+
expect(request({ url: '/users' }).method).toEqual('GET');
92+
});
93+
it('should default to POST with data', () => {
94+
expect(
95+
request({
96+
url: '/users',
97+
jsonData: {
98+
userId: 123,
99+
},
100+
}).method
101+
).toEqual('POST');
102+
});
103+
it('should accept GET with data', () => {
104+
expect(
105+
request({
106+
url: '/users',
107+
method: 'GET',
108+
jsonData: {
109+
userId: 123,
110+
},
111+
}).method
112+
).toEqual('GET');
113+
});
114+
it('should accept any method', () => {
115+
expect(request({ url: '/users', method: 'DELETE' }).method).toEqual('DELETE');
116+
expect(request({ url: '/users', method: 'PUT' }).method).toEqual('PUT');
117+
expect(request({ url: '/users', method: 'BEEP' }).method).toEqual('BEEP');
118+
});
136119
});
137120

138-
it('should use the first filename specified', () => {
139-
expect(getFilenameFromContentDisposition('attachment; filename*=data.xlsx; filename=other.csv')).toBe(
140-
'data.xlsx'
141-
);
142-
expect(getFilenameFromContentDisposition('attachment; filename=data.xlsx; filename*=other.csv')).toBe(
143-
'data.xlsx'
144-
);
145-
expect(getFilenameFromContentDisposition("attachment; filename*=UTF-8''d%20ata.xlsx; filename=other.csv")).toBe(
146-
'd ata.xlsx'
147-
);
121+
describe('getFilenameFromContentDisposition', () => {
122+
it('should not find a file if there is no attachment prefix', () => {
123+
expect(getFilenameFromContentDisposition('other; filename=data.tsv')).toBeUndefined();
124+
expect(getFilenameFromContentDisposition('other; filename=attachment.tsv')).toBeUndefined();
125+
});
126+
127+
it('should not find a file if there is not a valid filename prefix', () => {
128+
expect(getFilenameFromContentDisposition('attachment; filename=d ata.xl sx')).toBeUndefined();
129+
expect(getFilenameFromContentDisposition('attachment; filename*=data.tsv')).toBeUndefined();
130+
expect(getFilenameFromContentDisposition('attachment; filename*=attachment.tsv')).toBeUndefined();
131+
});
132+
133+
it('should find a file with the filename= prefix', () => {
134+
expect(getFilenameFromContentDisposition('attachment; filename=data.xlsx')).toBe('data.xlsx');
135+
expect(getFilenameFromContentDisposition('attachment; filename="d ata.xl sx"')).toBe('d ata.xl sx');
136+
expect(getFilenameFromContentDisposition('attachment;filename="plans.pdf" \t; \t\t foo=bar')).toBe(
137+
'plans.pdf'
138+
);
139+
expect(getFilenameFromContentDisposition('attachment; filename="£ rates.pdf"')).toBe('£ rates.pdf');
140+
});
141+
142+
it('should find a file with the filename*= prefix', () => {
143+
expect(
144+
getFilenameFromContentDisposition("attachment; filename=data.xlsx; filename*=UTF-8''datastar.xlsx")
145+
).toBe('datastar.xlsx');
146+
expect(
147+
getFilenameFromContentDisposition('attachment; filename="d ata.xl sx"; filename*=utf-8\'\'d%20ata.xlsx')
148+
).toBe('d ata.xlsx');
149+
expect(getFilenameFromContentDisposition("attachment; filename*=UTF-8''%E2%82%AC%20rates.pdf")).toBe(
150+
'€ rates.pdf'
151+
);
152+
expect(
153+
getFilenameFromContentDisposition("attachment; filename*=utf-8''data.xlsx; filename=other.csv")
154+
).toBe('data.xlsx');
155+
expect(
156+
getFilenameFromContentDisposition("attachment; filename=data.xlsx; filename*=utf-8''other.csv")
157+
).toBe('other.csv');
158+
expect(
159+
getFilenameFromContentDisposition("attachment; filename*=UTF-8''d%20ata.xlsx; filename=other.csv")
160+
).toBe('d ata.xlsx');
161+
// Issue 50628: verify content-disposition parsing of filenames that include encoded characters
162+
expect(
163+
getFilenameFromContentDisposition(
164+
'attachment; filename="=?UTF-8?Q?ELN-1005-20240903-5523_London,_England.pdf?="; filename*=UTF-8\'\'ELN-1005-20240903-5523%20London%2C%20England.pdf'
165+
)
166+
).toBe('ELN-1005-20240903-5523 London, England.pdf');
167+
});
148168
});
149169
});

0 commit comments

Comments
 (0)