Skip to content
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

Add workaround for sockjs@~0.3.19 #1608

Merged
merged 4 commits into from
Dec 24, 2018

Conversation

3846masa
Copy link
Member

@3846masa 3846masa commented Dec 22, 2018

  • 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

sockjs will remove Origin header, however Origin header is required for checking host (#1603).
See #1604 (comment)

This workaround overwrote Session.prototype.decorateConnection.
(See https://github.com/sockjs/sockjs-node/blob/v0.3.19/src/transport.coffee#L112-L140)

Breaking Changes

Additional Info

This workaround maybe broken at sockjs@^0.4.x because sockjs are refactoring now.
When sockjs/sockjs-node#247 is merged, this workaround is not necessary.

sockjs will remove Origin header, however Origin header is required for checking host.
This workaround maybe broken at sockjs@^0.14.x because of refactoring.
@3846masa
Copy link
Member Author

I'll fix code style. sorry

@codecov
Copy link

codecov bot commented Dec 22, 2018

Codecov Report

Merging #1608 into master will decrease coverage by 0.32%.
The diff coverage is 42.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1608      +/-   ##
==========================================
- Coverage   74.25%   73.93%   -0.33%     
==========================================
  Files          10       10              
  Lines         672      679       +7     
==========================================
+ Hits          499      502       +3     
- Misses        173      177       +4
Impacted Files Coverage Δ
lib/Server.js 81.01% <42.85%> (-0.73%) ⬇️

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 bddfe16...f55088a. Read the comment docs.

@lbogdan
Copy link
Contributor

lbogdan commented Dec 22, 2018

Re: PR title, it's 0.3.19, not 0.13.9.

@lbogdan
Copy link
Contributor

lbogdan commented Dec 22, 2018

I can confirm this fixes the websocket origin checking issue with projects created by @vue/cli: #1604 (comment) .

@alexander-akait
Copy link
Member

@lbogdan Thanks!

@3846masa 3846masa changed the title Add workaround for sockjs@~0.13.9 Add workaround for sockjs@~0.3.9 Dec 22, 2018
@3846masa 3846masa changed the title Add workaround for sockjs@~0.3.9 Add workaround for sockjs@~0.3.19 Dec 22, 2018
@DannyFeliz
Copy link

Yes, this definitely fixes the issue.
Nice job 👍

@anchengjian
Copy link

It's working for me, @3846masa thanks

@rafamp85
Copy link

Good night everybody,

I don't know how to apply this workaround in my Angular project, what do I have to do.

BR

@alexander-akait alexander-akait merged commit 1dfd4fb into webpack:master Dec 24, 2018
@bradharms
Copy link

It seems like I'm still getting this even after installing webpack-dev-server@^3.1.14. I am not using Angular or Vue or any other framework, just React and react-hot-loader.

@ChuckJonas
Copy link

ChuckJonas commented Jan 2, 2019

Also still getting this error with 3.1.14 & react-hot-loader

@rcapile
Copy link

rcapile commented Jul 24, 2020

Also still getting this error with 3.1.14 & symfony proxy. I solved with

Encore.configureDevServerOptions(options => {
    options.allowedHosts = ['my-domain.wip']
})

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.

10 participants