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

Fix for #535 ExtraHeaders doesn't work in browser #536

Merged
merged 3 commits into from
Feb 1, 2017
Merged

Fix for #535 ExtraHeaders doesn't work in browser #536

merged 3 commits into from
Feb 1, 2017

Conversation

lu4
Copy link
Contributor

@lu4 lu4 commented Jan 30, 2017

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

xhr.setDisableHeaderCheck is not introduced in some browsers so engine.io fails to set extraHeaders which makes feature #519 to fail.

New behaviour

xhr.setDisableHeaderCheck is checked to exist before call, in case if any secure headers are modified, browser throws error to prevent fraud.

Other information (e.g. related issues)

Please provide feedback in case if any PR rules are violated

@darrachequesne darrachequesne merged commit 22c28a2 into socketio:master Feb 1, 2017
@darrachequesne
Copy link
Member

Great catch, thanks! I've just reverted the change to the output file engine.io.js, as it is automatically generated.

@lu4
Copy link
Contributor Author

lu4 commented Feb 1, 2017

Thanks, yeah I was unsure if that was necessary move, so created a separate commit,

Regards,
Denis

@darrachequesne
Copy link
Member

@lu4 out of curiosity, in your example here:

let engine = engineio({
    transportOptions: {
        polling: {
            extraHeaders: {
                'X-TEST': 'ZZZ'
            }
        }
    }
});

Won't the X-TEST header generate an OPTION request, which will result in a 400 response from the engine.io server? (I'm wondering if there is something to fix on our side, ref: socketio/engine.io#279)

@lu4
Copy link
Contributor Author

lu4 commented Feb 1, 2017

It wasn't my case, everything were working smoothly, no option request was generated... I didn't mean to use any predefined headers, just a first thing that came of my head...

@darrachequesne darrachequesne added this to the 2.0.2 milestone Feb 16, 2017
sgress454 added a commit to sgress454/socket.io-client that referenced this pull request Feb 16, 2017
Includes the following (from engine.io-client changelog):

* [chore] Bump ws to version 1.1.2 (vulnerability fix) ([socketio#539](socketio/engine.io-client#539))
* [fix] Fix extraHeaders option in browser ([socketio#536](socketio/engine.io-client#536))
darrachequesne pushed a commit to socketio/socket.io-client that referenced this pull request Feb 16, 2017
Includes the following (from engine.io-client changelog):

* [chore] Bump ws to version 1.1.2 (vulnerability fix) ([#539](socketio/engine.io-client#539))
* [fix] Fix extraHeaders option in browser ([#536](socketio/engine.io-client#536))
enderson-pan pushed a commit to holytiny/feathersjs-wxmp-socket.io-client that referenced this pull request Nov 1, 2019
Includes the following (from engine.io-client changelog):

* [chore] Bump ws to version 1.1.2 (vulnerability fix) ([#539](socketio/engine.io-client#539))
* [fix] Fix extraHeaders option in browser ([#536](socketio/engine.io-client#536))
sunrise30 added a commit to sunrise30/socket.io-client that referenced this pull request Jan 8, 2022
Includes the following (from engine.io-client changelog):

* [chore] Bump ws to version 1.1.2 (vulnerability fix) ([#539](socketio/engine.io-client#539))
* [fix] Fix extraHeaders option in browser ([#536](socketio/engine.io-client#536))
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.

2 participants