Skip to content

Commit

Permalink
fix: skip content-type header on unknown types
Browse files Browse the repository at this point in the history 
Fixes webpack#354

* Only set 'Content-Type' header if mime type is known.
* Remove testing for 'Content-Type' of unknown types.
* Do not test against `application/octet-stream'.

Unknown media types should have content-type blank. Because
frameworks handle Content-Types differently, it is not possible to
standardize testing at the moment. Tests against
`application/octet-stream` can create false positives with Express
because of this.

https://tools.ietf.org/html/rfc7231#section-3.1.1.5
broofa/mime#139
  • Loading branch information
@clshortfuse
clshortfuse committed Jan 1, 2021
1 parent b1fe6bd commit ee7e198
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 67 deletions.
21 changes: 11 additions & 10 deletions src/middleware.js
View file
Expand Up @@ -65,16 +65,17 @@ export default function wrapper(context) {
// content-type name(like application/javascript; charset=utf-8) or false
const contentType = mime.contentType(path.extname(filename));

// Express API
if (res.set && contentType) {
res.set('Content-Type', contentType);
}
// Node.js API
else {
res.setHeader(
'Content-Type',
contentType || 'application/octet-stream'
);
// Only set content-type header if media type is known
// https://tools.ietf.org/html/rfc7231#section-3.1.1.5
if (contentType) {
// Express API
if (res.set) {
res.set('Content-Type', contentType);
}
// Node.js API
else {
res.setHeader('Content-Type', contentType);
}
}
}

Expand Down
78 changes: 21 additions & 57 deletions test/middleware.test.js
View file
Expand Up @@ -290,7 +290,6 @@ describe.each([
request(app)
.get('/unknown')
.expect('Content-Length', fileData.byteLength.toString())
.expect('Content-Type', 'application/octet-stream')
.expect(200, done);
});
});
Expand Down Expand Up @@ -584,7 +583,6 @@ describe.each([
urls: [
{
value: 'filename-name-with-dots/mono-v6.x.x',
contentType: 'application/octet-stream',
code: 200,
},
],
Expand All @@ -595,7 +593,6 @@ describe.each([
urls: [
{
value: 'noextension',
contentType: 'application/octet-stream',
code: 200,
},
],
Expand Down Expand Up @@ -812,12 +809,19 @@ describe.each([
it(`should return the "${code}" code for the "GET" request for the "${value}" url`, (done) => {
request(app)
.get(`${publicPathForRequest}${value}`)
.expect('Content-Type', contentType)
.expect(
'Content-Length',
data ? String(data.length) : /\d+/
)
.expect(code, done);
.expect(code)
.then((res) => {
if (contentType) {
expect(res.headers['content-type']).toEqual(
contentType
);
}
})
.then(done);
});
}
}
Expand All @@ -835,11 +839,14 @@ describe.each([
app.use((req, res, next) => {
// Express API
if (res.set) {
res.set('Content-Type', 'application/octet-stream');
res.set('Content-Type', 'application/vnd.test+octet-stream');
}
// Connect API
else {
res.setHeader('Content-Type', 'application/octet-stream');
res.setHeader(
'Content-Type',
'application/vnd.test+octet-stream'
);
}
next();
});
Expand All @@ -850,10 +857,10 @@ describe.each([

afterAll(close);

it('should not guess a MIME type if the "Content-Type" header is found', (done) => {
it('should not modify the "Content-Type" header', (done) => {
request(app)
.get('/bundle.js')
.expect('Content-Type', 'application/octet-stream')
.expect('Content-Type', 'application/vnd.test+octet-stream')
.expect(200, done);
});
});
Expand Down Expand Up @@ -2019,43 +2026,6 @@ describe.each([
});
});

describe('should set the correct value for "Content-Type" header to unknown MIME type', () => {
beforeAll((done) => {
const outputPath = path.resolve(__dirname, './outputs/basic');
const compiler = getCompiler({
...webpackConfig,
output: {
filename: 'bundle.js',
path: outputPath,
},
});

instance = middleware(compiler);

app = framework();
app.use(instance);

listen = listenShorthand(done);

instance.context.outputFileSystem.mkdirSync(outputPath, {
recursive: true,
});
instance.context.outputFileSystem.writeFileSync(
path.resolve(outputPath, 'file.phtml'),
'welcome'
);
});

afterAll(close);

it('should return the "200" code for the "GET" request to "file.phtml"', (done) => {
request(app)
.get('/file.phtml')
.expect('Content-Type', 'application/octet-stream')
.expect(200, done);
});
});

describe('should set the correct value for "Content-Type" header to specified MIME type', () => {
beforeAll((done) => {
const outputPath = path.resolve(__dirname, './outputs/basic');
Expand Down Expand Up @@ -2110,7 +2080,7 @@ describe.each([

instance = middleware(compiler, {
mimeTypes: {
jpg: 'application/octet-stream',
jpg: 'image/vnd.test+jpeg',
},
});

Expand All @@ -2133,7 +2103,7 @@ describe.each([
it('should return the "200" code for the "GET" request "file.jpg"', (done) => {
request(app)
.get('/file.jpg')
.expect('Content-Type', /application\/octet-stream/)
.expect('Content-Type', 'image/vnd.test+jpeg')
.expect(200, done);
});
});
Expand All @@ -2151,7 +2121,7 @@ describe.each([

instance = middleware(compiler, {
mimeTypes: {
jpg: 'application/octet-stream',
jpg: 'image/vnd.test+jpeg',
},
});

Expand Down Expand Up @@ -3225,10 +3195,7 @@ describe.each([
afterAll(close);

it('should return the "200" code for the "GET" request to the public path', (done) => {
request(app)
.get('/')
.expect('Content-Type', 'application/octet-stream')
.expect(200, done);
request(app).get('/').expect(200, done);
});
});

Expand Down Expand Up @@ -3305,10 +3272,7 @@ describe.each([
afterAll(close);

it('should return the "200" code for the "GET" request to the public path', (done) => {
request(app)
.get('/')
.expect('Content-Type', 'application/octet-stream')
.expect(200, done);
request(app).get('/').expect(200, done);
});
});

Expand Down

0 comments on commit ee7e198

Please sign in to comment.