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: improve proto checking for hgetall [skip ci] #1418

Merged
merged 2 commits into from
Aug 18, 2021
Merged

Conversation

luin
Copy link
Collaborator

@luin luin commented Aug 18, 2021

As mentioned in #1417

@luin luin changed the title fix: improve proto checking for hgetall fix: improve proto checking for hgetall [skip ci] Aug 18, 2021
Copy link
Collaborator

@TysonAndre TysonAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, documentation of https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/in#parameters in operator is what I'd expect it to be

expect(ret[CUSTOM_PROPERTY]).to.eql("world");
expect(Object.keys(ret).sort()).to.eql(
["__proto__", CUSTOM_PROPERTY].sort()
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this change: Could also check that expect(Object.getPrototypeOf(ret)).to.eql(Object.prototype); to confirm the original prototype isn't overridden, though the Object.keys() check pretty much implies that

@luin luin merged commit cba83cb into master Aug 18, 2021
@luin luin deleted the hgetall-key-in-obj branch August 18, 2021 16:48
ioredis-robot pushed a commit that referenced this pull request Aug 30, 2021
## [4.27.9](v4.27.8...v4.27.9) (2021-08-30)

### Bug Fixes

* Fix undefined property warning in executeAutoPipeline ([#1425](#1425)) ([f898672](f898672))
* improve proto checking for hgetall [skip ci] ([#1418](#1418)) ([cba83cb](cba83cb))
@ioredis-robot
Copy link
Collaborator

🎉 This PR is included in version 4.27.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

janus-dev87 added a commit to janus-dev87/ioredis-work that referenced this pull request Mar 1, 2024
## [4.27.9](redis/ioredis@v4.27.8...v4.27.9) (2021-08-30)

### Bug Fixes

* Fix undefined property warning in executeAutoPipeline ([#1425](redis/ioredis#1425)) ([f898672](redis/ioredis@f898672))
* improve proto checking for hgetall [skip ci] ([#1418](redis/ioredis#1418)) ([cba83cb](redis/ioredis@cba83cb))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants