-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(Server): validate express.static.mime.types #1765
Conversation
9e1ea37
to
4e5af29
Compare
lib/Server.js
Outdated
express.static && | ||
express.static.mime && | ||
express.static.mime.types && | ||
!express.static.mime.types.wasm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think better to remove !express.static.mime.types.wasm
because in some old cases it can be text/html
and it is invalid, we need use right mime for wasm
4e5af29
to
1cdb254
Compare
Codecov Report
@@ Coverage Diff @@
## master #1765 +/- ##
==========================================
+ Coverage 87.6% 87.62% +0.02%
==========================================
Files 9 9
Lines 589 590 +1
Branches 175 176 +1
==========================================
+ Hits 516 517 +1
Misses 61 61
Partials 12 12
Continue to review full report at Codecov.
|
@evilebottnawi PTAL 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job 👍
/cc @hiroppy something wrong with linting |
1cdb254
to
2d0a572
Compare
Fixed. |
God job! |
fixes: #1724
For Bugs and Features; did you add new tests?
Yes.
A test which Checks
application/wasm
already exists.https://github.com/webpack/webpack-dev-server/blob/master/test/ContentBase.test.js#L301
Motivation / Use-Case
Validate express object.
Breaking Changes
no
Additional Info
no