Do not overwrite Content-Type #377
Conversation
When the Content-Type header is already present (for example it has been set by other middleware) we do not overwrite it. Closes webpack#376
|
|
Codecov Report
@@ Coverage Diff @@
## master #377 +/- ##
==========================================
+ Coverage 96.95% 96.96% +0.01%
==========================================
Files 7 7
Lines 263 264 +1
==========================================
+ Hits 255 256 +1
Misses 8 8
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #377 +/- ##
==========================================
+ Coverage 96.95% 96.96% +0.01%
==========================================
Files 7 7
Lines 263 264 +1
==========================================
+ Hits 255 256 +1
Misses 8 8
Continue to review full report at Codecov.
|
| res.setHeader('Content-Type', contentType); | ||
| if (!res.getHeader('Content-Type')) { | ||
| res.setHeader('Content-Type', contentType); | ||
| } | ||
| res.setHeader('Content-Length', content.length); |
There was a problem hiding this comment.
What do you think should we add check here too? I think in theory it is also possible to set own Content-Length, don't known real use case, but still think what we should add check here too
There was a problem hiding this comment.
This was my initial thought too. But I think matters are more complicated... It looks like ExpressJS itself sets the Content-Length
In fact line 85 seems to be not functional at all.
There was a problem hiding this comment.
@dmohns good catch, can you create issue? Will be great if you help to use investigate this, let's merge this as is
There was a problem hiding this comment.
hello，this commit make error，"TypeError: res.getHeader is not a function"，my node version is v10.15.2，how can i solve this problem
There was a problem hiding this comment.
👍 Will do.
hello，this commit make error，"TypeError: res.getHeader is not a function"，my node version is v10.15.2，how can i solve this problem
|
Unfortunately, I just realized the test in this PR is "wrong". The headers get set after Content-Type already got overwritten. Hence we are not really testing the scope of this PR. I'll shoot a separate PR with a fix. |
|
@dmohns 👍 |
What kind of change does this PR introduce?
When the Content-Type header is already present (for example it has been
set by other middleware) we do not overwrite it.
Did you add tests for your changes?
Yes, added unit test to the suite.
Summary
Closes #376
Does this PR introduce a breaking change?
No
Other information