Don't rely on strict mode behaviour for arguments

[emilbroman-eqt]

Currently, the main axios.request method/axios function relies on JavaScript's Strict Mode's behaviour of arguments. Here's the relevant MDN section

This 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 config being reassigned to (effectively) {} changes the value of arguments[0], which is exactly the behaviour that strict mode changes.

Subsequently, the config.url = arguments[0]; statement effectively does config.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 😄

[chinesedfan]

this behaviour isn't maintained by Node v14.15.1.

Can you describe the different behaviour by another simple Node.js script? Something like,

function fn(config) {
  config = arguments[1] || {}
  console.log(arguments[0]) // 'test string' in strict mode, and `{}` in Node v14.15.1?
}

fn('test string')

[emilbroman-eqt]

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 "use strict" statement is not longer the first in the function, making it invalid.

So, the issue is not with axios, but rather with pino-debug, since it invalidates all "use strict" statements in the top of all modules.

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 arguments at all, making this a non-issue.

I will leave it up to you whether to merge this or not.

Thanks 👋

[chinesedfan]

strict mode code doesn't alias properties of arguments objects created within it

I see.