joewalnes / reconnecting-websocket

A small decorator for the JavaScript WebSocket API that automatically reconnects
MIT License
4.21k stars 970 forks source link

Add setProtocols() to enable dynamic protocols. Added minification script for development. #104

Open bnielsen1965 opened 5 years ago

bnielsen1965 commented 5 years ago

I have a use case where a JSON Web Token is included in the protocols for websocket authentication. When the token expires and is renewed a method was needed to set the websocket protocol with the new token for any future reconnects.

So I added the setProtocols() method that enables dynamic changes to the protocols used in reconnects.

I also added an npm script and development dependency to simplify the minification step. You can do an npm install to load the uglify script and then npm run minifi to create the minified version.

alberto-f commented 5 years ago

Im not owner of this repo. Said that, I would recommend to avoid MR with minified version. Releases should handle offering a minified version.

bnielsen1965 commented 5 years ago

@alberto-f What is MR?

I'm a bit slow this morning and I'm not sure I understand. Are you saying the PR should not include the minified version but should be created during the release process? Would that require an additional npm script in package.json?

alberto-f commented 5 years ago

MR ( merge request) but yours is a PR( pull request). My bad.

@bnielsen1965 My point was that the owner of the library should provide the minified version with every new release of the library. Maybe even some hash for the minified version, so others can check the integrity of the minified version.

Package.json having a script command for minify the library is the usual.

I'm assuming that owners of libraries will not embedded malicious stuff in their libraries.

If non-owners provide minified versions of the library without a hash to check for integrity, owners cannot really track if there is some malicious code in the minified version.

Hopefully the owner of this library can tell you how to proceed. Mine was just a suggestion for security.

makyen commented 5 years ago

@alberto-f While what you're getting at is a good idea, there are some things that aren't that great in either the process you envision or in how you are describing it.

The integrity of the minimized version that's in a repository such as this is ensured by there being a predefined, deterministic, programmatic way to generate the minified version from the non-minified version. This allows the integrity of minified version to be checked by anyone who desires to do so, just by running the procedure on the non-minimized code and comparing the result with what is in either the PR or in the repository (depending on what's being checked).

If a PR includes a minified version, then the person merging the PR should perform that check prior to merging. There's no need for a hash to be provided with the PR for the purpose of verifying the minified version in the PR. The complete minified version that's in the PR is immediately available and the complete check version has to be generated anyway. Thus, a direct, byte-for-byte comparison can be done without the need for a hash. Doing a byte-for-byte comparison is better than using a hash, as it's possible to have the same hash for two different files. Such collisions of hashes is extremely unlikely to happen randomly, but is an attack vector. A direct byte-for-byte comparison doesn't have even that possible vector.

OTOH, it is desirable for there to be a hash provided by the project that's obtained separately from a download, so people can verify any download, and/or use resource integrity settings when loading from a CDN. This is particularly true for situations where you're not obtaining the download from the original source here on GitHub.

bnielsen1965 commented 5 years ago

@alberto-f @makyen I added a release and verify script to package.json that use the minifi script to generate the minified release file and to verify the minified version can be reproduced from the source.

I considered also spitting out a hash but wasn't having a lot of luck with any of the cli npm modules matching sha sum methods in a linux command line.