ant-design / ant-design

An enterprise-class UI design language and React UI library
https://ant.design
MIT License
91.87k stars 48.81k forks source link

Antd Less creates global JavaScript leaks #11097

Closed matthew-dean closed 1 year ago

matthew-dean commented 6 years ago

Version

3.6.4

Environment

Mac Node.js

Reproduction link

https://github.com/ant-design/ant-design

Steps to reproduce

I'm a maintainer of Less. There were some reported issues of using Antd with Less 3.x. I decided to try adding ant-design to Less tests. But immediately a global leak was detected by Less tests. I got the error message: Global leak detected: colorEasing, tinycolor, colorPalette

It seems that the Less files, when Inline JavaScript is enabled, add functions to the global object.

What is expected?

Less style files should not manipulate or leak into the global object.

What is actually happening?

colorEasing, tinycolor, colorPalette are leaking globally.


If you need help with creating a proper Less plugin (Inline JavaScript is discouraged for security and leak reasons like this), you can ask questions here: https://gitter.im/less/less.js

yutingzhao1991 commented 6 years ago

cc @chenshuai2144

tylik1 commented 5 years ago

Shouldn't this be fixed ASAP, if it is a security vulnerability issue? My build is failing when i try to import less file with webpack, because enableJavascript is set to false by default.

Probably this is the proper way of including js now? http://lesscss.org/features/#plugin-atrules-feature

matthew-dean commented 5 years ago

@tylik1 This isn't directly related to the inline JavaScript issue. This is what that inline JavaScript is doing (leaking into the global namespace), which is its own problem. So this isn't so much a security vulnerability issue as a performance / memory issue. Antd shouldn't be calling "Less" code that then has embedded JS code that is injecting vars globally into the Node runtime. There are layers of bad practices here.

kaiyoma commented 5 years ago

Are there any updates here? Or on the topic of Ant's use of inline JavaScript in general? Our project is evaluating Ant and we just ran into this as well. Ant seems promising, but we're nervous about the fact that it's using a deprecated, insecure LESS feature. Would like to know if there is a timeline or roadmap for fixing Ant's stylesheet to not use this mechanism.

rmadsen commented 5 years ago

It looks this was addressed (at least partially) in https://github.com/ant-design/ant-design/pull/13595 which was then reverted the next day by @chenshuai2144 in https://github.com/ant-design/ant-design/pull/13613, then that revert was reverted in https://github.com/ant-design/ant-design/pull/13621, and then that was reverted in https://github.com/ant-design/ant-design/pull/13622, leaving us back in the original state.

I don't see any conversation about what was wrong with the original merge -- @chenshuai2144 do you have any details about what would need to be changed in order for us to resolve this issue, or what caused the flurry of reversions?

afc163 commented 4 years ago

Another reason to remove javascriptEnable of less: #19234

biw-joelschou commented 3 years ago

Is the antd team bailing on this issue? I see the PR to resolve was closed rather unceremoniously (https://github.com/ant-design/ant-design/pull/20470) and switching javascriptEnabled to false in my webpack.config throws errors for .bezierEasingMixin();.

ixowuif commented 3 years ago

any updates on this?

connorhvnsen commented 2 years ago

Any updates here?

zombieJ commented 1 year ago

Close since v5 use cssinjs instead. No need less anymore.