Skip to content

Conversation

@timfjord
Copy link

  • 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?

No

Motivation / Use-Case

This is a port of #1603 fix but for v2 branch.
Basically, it addresses CVE vulnerability https://www.npmjs.com/advisories/725

I know that v2 is deprecated but it is not an easy task to upgrade this lib to v3 in rails apps, because some dependencies are still in release candidate state or even in beta.

Breaking Changes

Websocket is now checked for origin

Additional Info

@jsf-clabot
Copy link

jsf-clabot commented Jan 14, 2019

CLA assistant check
All committers have signed the CLA.

@hiroppy
Copy link
Member

hiroppy commented Feb 14, 2019

@Timsly please sign 👆

@alexander-akait
Copy link
Member

Tests failed, need fix

@timfjord
Copy link
Author

@hiroppy Done, signed
@evilebottnawi here is what I see in the build
14 48 47

@alexander-akait
Copy link
Member

Yep, expected, we can merge this and release, don't have access for computer right now

@timfjord
Copy link
Author

That would be great

@hiroppy
Copy link
Member

hiroppy commented Mar 5, 2019

rebuild https://ci.appveyor.com/project/sokra/webpack-dev-server

@Timsly Could you rebase from master?

@timfjord
Copy link
Author

timfjord commented Mar 6, 2019

@hiroppy I cannot rebase from master because this PR is exclusively for v2 and master contains v3

@alexander-akait
Copy link
Member

/cc @Timsly due we don't have tests for v2 so we should test this manually, do you check you solution is work as expected? Because we merge 8bb3ca8 and 1dfd4fb after original PR to avoid some problems

@timfjord
Copy link
Author

timfjord commented Mar 6, 2019

Hm, in this case, I will cherry-pick them here

@timfjord
Copy link
Author

timfjord commented Mar 13, 2019

@evilebottnawi I've ported both fixes, see f6c6af6 and ff8b19c
Could you please run the build to make sure it works as expected?

@alexander-akait
Copy link
Member

@Timsly we don't have tests for v2 so i can't, can you test this manually? Maybe ping something else for testing too

@timfjord
Copy link
Author

@evilebottnawi I've tested current version and it works fine.

@alexander-akait alexander-akait merged commit c42d0da into webpack:v2 Mar 22, 2019
@timfjord timfjord deleted the CVE-2018-14732-v2 branch March 22, 2019 14:15
@timfjord
Copy link
Author

@evilebottnawi is there a chance that this will be released as 2.11.2?

@alexander-akait
Copy link
Member

@Timsly today

@timfjord
Copy link
Author

Thanks

// if hostHeader doesn't have scheme, add // for parsing.
/^(.+:)?\/\//.test(hostHeader) ? hostHeader : `//${hostHeader}`,
false,
true,

Choose a reason for hiding this comment

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

@Timsly, would you mind to delete this comma with another PR? It causes issues for node.js <= 7.x
See issue #1607 and the PR that fixed it #1609

Copy link
Author

Choose a reason for hiding this comment

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

@ifnoword Done - #1736

Choose a reason for hiding this comment

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

@Timsly I appreciate it. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants