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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

hasOwnProperty was exported accidentally from the package #3158

Closed
iegik opened this issue Feb 27, 2024 · 8 comments
Closed

hasOwnProperty was exported accidentally from the package #3158

iegik opened this issue Feb 27, 2024 · 8 comments

Comments

@iegik
Copy link
Contributor

iegik commented Feb 27, 2024

Hi! 馃憢

Firstly, thanks for your work on this project! 馃檪

Today I used patch-package to patch @emotion/react@11.11.3 for the project I'm working on.

I found that hasOwnProperty was exported accidentally:

var hasOwnProperty = {}.hasOwnProperty;
...
exports.hasOwnProperty = hasOwnProperty;

Here is the diff that solved my problem:

diff --git a/node_modules/@emotion/react/dist/emotion-element-48d2c2e4.cjs.dev.js b/node_modules/@emotion/react/dist/emotion-element-48d2c2e4.cjs.dev.js
index 9836e58..2c3b083 100644
--- a/node_modules/@emotion/react/dist/emotion-element-48d2c2e4.cjs.dev.js
+++ b/node_modules/@emotion/react/dist/emotion-element-48d2c2e4.cjs.dev.js
@@ -306,7 +306,6 @@ exports.ThemeContext = ThemeContext;
 exports.ThemeProvider = ThemeProvider;
 exports.__unsafe_useEmotionCache = __unsafe_useEmotionCache;
 exports.createEmotionProps = createEmotionProps;
-exports.hasOwnProperty = hasOwnProperty;
 exports.isBrowser = isBrowser;
 exports.useTheme = useTheme;
 exports.withTheme = withTheme;

This issue body was partially generated by patch-package.

@iegik
Copy link
Contributor Author

iegik commented Feb 27, 2024

Fix added #3159

@Andarist
Copy link
Member

You are not mentioning what the problem is though. I don't see how this would be relevant for the final consumer (like you). It's an internal file.

@iegik
Copy link
Contributor Author

iegik commented Feb 27, 2024

Problem is that hasOwnProperty already exists in exports object. The emotion/react bundle tries override the method with new one. This is dumb and can be omitted by this patch. See other packages in the same project, where hasOwnProperty where used. ({}.hasOwnProperty)

For-more, since exports is actually an object, it has already exports the .hasOwnProperty property through the prototype.

@Andarist
Copy link
Member

For-more, since exports is actually an object, it has already exports the .hasOwnProperty property through the prototype.

I'm aware of that. But what problem does this overridden method create here? We can freely override methods like this as long as it's not observable in a negative way.

@iegik
Copy link
Contributor Author

iegik commented Feb 27, 2024

Cut your finger and attach again. No problem, isn't it? :)

@Andarist
Copy link
Member

I'm sorry - I don't understand what you mean and I still don't see what the reported problem is. I'd appreciate if you'd focus on technical sides of things here.

@iegik
Copy link
Contributor Author

iegik commented Feb 27, 2024

To prevent changes to prototype objects I use Object.freeze(Object.prototype) as mentioned as a solution here: https://portswigger.net/web-security/prototype-pollution/preventing

It works with most other packages. And throws an error with emotion.

The error looks like this:

TypeError: Cannot assign to read only property 'hasOwnProperty' of object '[object Object]'

node_modules/@emotion/react/dist/emotion-react.cjs.dev.js:5:22

// var emotionElement = require('./emotion-element-48d2c2e4.cjs.dev.js');
// ...
// var hasOwnProperty = {}.hasOwnProperty;
// ...
// exports.hasOwnProperty = hasOwnProperty;

@Andarist
Copy link
Member

Andarist commented Feb 27, 2024

fixed by #3159

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

No branches or pull requests

2 participants