Don't rely on strict mode behaviour for arguments#3470
Don't rely on strict mode behaviour for arguments#3470jasonsaayman merged 4 commits intoaxios:masterfrom
Conversation
Can you describe the different behaviour by another simple Node.js script? Something like, |
|
I have more information on this. It turns out that the problem is in conjunction with pino-debug, which decorates Node's module wrapper function, adding statements before the module's code is executed. That way, the So, the issue is not with axios, but rather with pino-debug, since it invalidates all This PR can be rejected if you want, but I would still argue that relying on strict mode in this way is bad form. It's so easy to not use I will leave it up to you whether to merge this or not. Thanks 👋 |
chinesedfan
left a comment
There was a problem hiding this comment.
strict mode code doesn't alias properties of arguments objects created within it
I see.
Currently, the main
axios.requestmethod/axiosfunction relies on JavaScript's Strict Mode's behaviour ofarguments. Here's the relevant MDN sectionThis should be fine, since the file is, in fact, in strict mode.
However, we've discovered that for some reason in our application this behaviour isn't maintained by Node v14.15.1.
In the screenshot above, you can see how
configbeing reassigned to (effectively){}changes the value ofarguments[0], which is exactly the behaviour that strict mode changes.Subsequently, the
config.url = arguments[0];statement effectively doesconfig.url = config;making a circular data structure, leading to stack overflows down the line.I don't know why this is happening, and I can't make a test since the test environment correctly follows the strict mode behaviour. But, of course, if we don't rely on this particular behaviour, the problem goes away altogether.
Please consider merging this PR since this bug effectively injected a stack overflow bomb into our production environment, and I wouldn't want to see that happen to anyone else 😄