Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(Server): validate express.static.mime.types #1765

Merged
merged 1 commit into from
Apr 5, 2019

Conversation

hiroppy
Copy link
Member

@hiroppy hiroppy commented Apr 5, 2019

fixes: #1724

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

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

@hiroppy hiroppy requested a review from alexander-akait as a code owner April 5, 2019 14:52
@hiroppy hiroppy force-pushed the feature/fix-server branch from 9e1ea37 to 4e5af29 Compare April 5, 2019 14:54
lib/Server.js Outdated
express.static &&
express.static.mime &&
express.static.mime.types &&
!express.static.mime.types.wasm
Copy link
Member

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

@hiroppy hiroppy force-pushed the feature/fix-server branch from 4e5af29 to 1cdb254 Compare April 5, 2019 14:58
@codecov
Copy link

codecov bot commented Apr 5, 2019

Codecov Report

Merging #1765 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
lib/Server.js 83.33% <100%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b3cd4f...1cdb254. Read the comment docs.

@hiroppy
Copy link
Member Author

hiroppy commented Apr 5, 2019

@evilebottnawi PTAL 🙏

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job 👍

@alexander-akait
Copy link
Member

/cc @hiroppy something wrong with linting

@hiroppy hiroppy force-pushed the feature/fix-server branch from 1cdb254 to 2d0a572 Compare April 5, 2019 16:16
@hiroppy
Copy link
Member Author

hiroppy commented Apr 5, 2019

Fixed.

@alexander-akait
Copy link
Member

God job!

@alexander-akait alexander-akait merged commit 919ff77 into master Apr 5, 2019
@alexander-akait alexander-akait deleted the feature/fix-server branch April 5, 2019 16:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot set property 'wasm' of undefined. Mismatched mime versions?
2 participants