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.

Sign up for GitHub

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

Jump to bottom

zlib strategy constants not strictly enforced #10932

Closed
Trott opened this issue Jan 21, 2017 · 1 comment
Closed

zlib strategy constants not strictly enforced #10932

Trott opened this issue Jan 21, 2017 · 1 comment
Labels
zlib Issues and PRs related to the zlib subsystem.

Comments

@Trott
Copy link
Member

Trott commented Jan 21, 2017

  • Version: v8.0.0-pre
  • Platform: Darwin Kernel Version 16.3.0
  • Subsystem: zlib

zlib.constant.Z_RLE is 3 but code will happily accept "3" and work just fine.

Either we should add a test to confirm that strings continue to work, or else strictly enforce that only exactly the constants provided by zlib.constants are acceptable.
@Trott Trott added the zlib Issues and PRs related to the zlib subsystem. label Jan 21, 2017
Trott added a commit to Trott/io.js that referenced this issue Jan 21, 2017
@Trott
zlib: be strict about what strategies are accepted
72590d7 
Currently, strategy constants are integers but Node.js will accept
string versions of those integers. Users should be using the provided
zlib constants and not hardcoding numbers, strings, or anything else. As
such, Node.js should be strict about accepting only exactly those values
that are in the provided zlib constants.

Fixes: nodejs#10932
Trott added a commit to Trott/io.js that referenced this issue Jan 21, 2017
@Trott
test,zlib: check string strategies
bf18ed8 
The Zlib constructor accepts strategy constants that are integers.
Current code will also work with string versions of those integers. Add
test for this situation.

Fixes: nodejs#10932
@Trott Trott mentioned this issue Jan 21, 2017
Closed
3 tasks
@Trott
Copy link
Member Author

Trott commented Jan 21, 2017

#10933 and #10934 are the two possible approaches to this issue.

@Trott Trott closed this as completed in dd928b0 Jan 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

zlib: be strict about what strategies are accepted Trott/io.js
1 participant
@Trott