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

[Deps] remove function-bind dependency #16

Closed
wants to merge 1 commit into from

Conversation

Conaclos
Copy link

@Conaclos Conaclos commented May 30, 2021

Hi!

Close to 90% of the bundle size is due to function-bind dependency. By removing this dependency, the package is lighter and we save bits for the planet :)

Close to 90% of the bundle size is due to `function-bind` dependency.
By removing this dependency, the package is lighter and we save bits
for the planet.
Copy link
Contributor

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

This causes the package to be broken when someone does delete Function.prototype.call later.

@zloirock
Copy link
Contributor

If someone will delete Function.prototype.call, your Node application will die anyway since it's used in Node, most likely your browser application will die too. Moreover, functions created by function-bind will not work - see the source code of function-bind implementation. And I still didn't say about delete Function.prototype.call before has loading.

@ljharb
Copy link
Contributor

ljharb commented May 31, 2021

There’s no defense from first-run code.

That node breaks in this case doesn’t change that browsers don’t, and this package (like most) is designed to also run in the browser.

@ljharb
Copy link
Contributor

ljharb commented May 31, 2021

There’s also no way to defend against .call deletion in an engine without a native bind - so the current state of the package is the best possible balance of robustness and correctness.

@ljharb ljharb closed this May 31, 2021
@Conaclos
Copy link
Author

@ljharb
I'm not sure to understand the reasoning...

In a sense my proposal reduces the attack surface.

In the current implementation, if Function.prototype.bind is undefined (potentially deleted), then the code can be "attacked" by redefining Array, Function, Object, Object.hasOwnProperty, Function.prototype.call, Function.prototype.apply, Array.prototype.concat, or Array.prototype.push, Array.prototype.slice. Otherwise the code can be "attacked" by redefining Object, Function, Object.hasOwnProperty, or Function.prototype.bind.

In contrast the proposed change reduces the attack surface at the redefinition or deletion of Object, Function, Object.hasOwnProperty, and Function.prototype.call.

@ljharb
Copy link
Contributor

ljharb commented Jun 1, 2021

It is always assumed that the module is evaluated in a clean realm (because nothing else is remotely defensible) - meaning, Function.prototype.bind either exists (native, or polyfilled) or does not. In the former case, function-bind just provides the native method. In the latter case, you’re right that https://unpkg.com/browse/function-bind@1.1.1/implementation.js has a number of attack surfaces at runtime - but the solution is to harden that package, not weaken this one.

@Conaclos
Copy link
Author

Conaclos commented Jun 1, 2021

@ljharb
Thanks for the explanation. This makes sense.

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.

None yet

3 participants