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 setProtocols() to enable dynamic protocols. Added minification script for development. #104

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bnielsen1965
Copy link

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
Copy link

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
Copy link
Author

@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
Copy link

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
Copy link

makyen commented Mar 19, 2019

@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
Copy link
Author

@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.

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.

3 participants