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

move things away from process.binding('util') #30884

Open
joyeecheung opened this issue Dec 10, 2019 · 3 comments
Open

move things away from process.binding('util') #30884

joyeecheung opened this issue Dec 10, 2019 · 3 comments
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Dec 10, 2019

process.binding('util') has been some kind of hotch-potch of "bindings the JS internals need but don't fall in the scope of a particular binding namespace". There is no compatibility guarantee about what's in this object and changes to it are completely at the whims of Node.js maintainers.

However process.binding('util') has been accessible in the user land and is abused by users (refs: #29947 (comment) - considering how volatile it is, when touching this binding I had always been wondering if anyone actually abuses this, until I saw the issue). I propose we try creating a new, internal namespace (that is, a namespace that's not in the whitelist here) and try moving everything away to that internal namespace to reduce the abuse.

On a side note, I think there should be a comment in node_utils.cc warning that any additions to the binding is at the risk of user-land abuse.

@juanarbol
Copy link
Member

I will work on this, do you have any specific name for that binding? And what methods should be on that binding.

@joyeecheung
Copy link
Member Author

@juanarbol I guess something like "internal_util" would work (though I am bad at naming stuff). I think you can start by git blaming the Initialize function in node_util.cc and starting from the most recent addition.

@BridgeAR BridgeAR added the lib / src Issues and PRs related to general changes in the lib or src directory. label Dec 20, 2019
@NotWearingPants
Copy link

Wasn't this fixed by #37819?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants