Skip to content

buffer: use private properties for brand checks in File#47154

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
KhafraDev:file-brand-checks
Mar 20, 2023
Merged

buffer: use private properties for brand checks in File#47154
nodejs-github-bot merged 1 commit intonodejs:mainfrom
KhafraDev:file-brand-checks

Conversation

@KhafraDev
Copy link
Member

@KhafraDev KhafraDev commented Mar 18, 2023

The current checks wouldn't work for certain values, and other places in the code have moved to this behavior as well.

import { File } from 'node:buffer'

const pd = Object.getOwnPropertyDescriptor(File.prototype, 'name')

pd.get.call(true) // Uncaught TypeError: Cannot use 'in' operator to search for '#name' in true

pd.get.call(null) // Uncaught TypeError [ERR_INVALID_THIS]: Value of "this" must be of type File

Refs: #46904

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Mar 18, 2023
@anonrig anonrig added semver-major PRs that contain breaking changes and should be released in the next major version. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Mar 18, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 18, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 18, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/50451/

@anonrig
Copy link
Member

anonrig commented Mar 19, 2023

Since, this change is done on an experimental API, I'll remove my tag of semver-major. If this is wrongly removed, please add it.

@anonrig anonrig removed the semver-major PRs that contain breaking changes and should be released in the next major version. label Mar 19, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 19, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/50455/

@github-actions github-actions bot mentioned this pull request Mar 20, 2023
Open
17 tasks
@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 20, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 20, 2023
@nodejs-github-bot nodejs-github-bot merged commit 252a069 into nodejs:main Mar 20, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 20, 2023

Landed in 252a069

@KhafraDev KhafraDev deleted the file-brand-checks branch March 20, 2023 22:22
@github-actions github-actions bot mentioned this pull request Mar 21, 2023
Open
19 tasks
@debadree25 debadree25 mentioned this pull request Mar 21, 2023
Closed
@github-actions github-actions bot mentioned this pull request Mar 22, 2023
Open
19 tasks
@anonrig anonrig mentioned this pull request Mar 26, 2023
Closed
@RafaelGSS RafaelGSS mentioned this pull request Apr 3, 2023
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
@KhafraDev @RafaelGSS
buffer: use private properties for brand checks in File
d639f2e 
PR-URL: #47154
Refs: #46904
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Apr 6, 2023
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
@KhafraDev @RafaelGSS
buffer: use private properties for brand checks in File
e3d98c3 
PR-URL: #47154
Refs: #46904
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
@KhafraDev @danielleadams
buffer: use private properties for brand checks in File
4c69be4 
PR-URL: #47154
Refs: #46904
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@danielleadams danielleadams mentioned this pull request Jul 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpinca lpinca lpinca approved these changes

@anonrig anonrig anonrig approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@KhafraDev @nodejs-github-bot @anonrig @lpinca