Strange behaviours when handling incorrect url

[PhantomRay]

Is your feature request related to a problem? Please describe.

Today I was fixing a small bug caused by incorrect use of url. I specified url without http:// or https://. It took me hours to figure it out. There were two behaviours from axios distracted me from identifying the real issue.

• axios tries to reach 127.0.0.1:80. I had no clue why it happened, at first I didn't know it was caused by axios. So I created a mock server to run locally on port 80 and see where it was coming from, then this issue was gone and I was hit by another issue,
• axios returns 400 error. This made me think the url I was calling returned 400.

To Reproduce:

const axios = require('axios');

axios({
  method: 'get',
  url: 'google.com/path', // url without https://
})
  .then(() => {
    // successful response
  })
  .catch((err) => {
    // when http://127.0.0.1:80 is not reachable, the err.message will be "connect ECONNREFUSED 127.0.0.1:80"
    // when http://127.0.0.1:80 is reachable, the err.message will be "Request failed with status code 400"
    console.error(err.message);
  });

Describe the solution you'd like

axios should throw proper error message (e.g.invalid url specified).

Describe alternatives you've considered

At the very least above behaviours should be avoided.

Additional context

• Axios Version [0.24.0]
• Node.js Version [14.x]
• OS: [any]

[ElmarFrerichs]

Hi,

this behaviour emegerges because deep down, axios uses the deprecated url.parse of the node url package:

axios/lib/adapters/http.js

Line 120 in c5fe05b

var parsed = url.parse(fullPath);

This fails if no protocol is specified, and puts the remainings of the parsed string somewhere in the result object, but not in the correct fields. Thus, the field hostname of the resulting parsed object is null, and seems to fallback to localhost in the deeper layers.

Url {
  protocol: null,
  slashes: null,
  auth: null,
  host: null,
  port: null,
  hostname: null,
  hash: null,
  search: null,
  query: null,
  pathname: 'google.com',
  path: 'google.com',
  href: 'google.com'
}

There are two possible ways of dealing with this:

• Catch an empty hostname and throw an error, as @PhantomRay proposed. This strategy might miss some other edge cases.
• Fix the deprecated call. Node docs suggest to use new URL(path) instead of url.parse(path). The new API throws an error Invalid URL for the missing protocol. However, the resulting URL object does not match the legacy object of url.parse, so this would require a bigger refactoring of the following code.

I'd like to take a shot at this together with @Arne117 (and add a unit test), but we are not yet sure which approach would work better. Any suggestions?

[PhantomRay]

Thank you very much for the detailed explanation.

[ElmarFrerichs]

Keeping in mind that @jasonsaayman plans to refactor the whole project with #4209, maybe the small solution just throwing an error would be sufficient until the refactoring of this code. Switching URL.parse() to new URL() will take quite some time and probably also create some new bugs in the process...

I'm thinking of a regex check like /^[^:]+:\/\//, throwing a new Error() on mismatch.

[PhantomRay]

Could we just specify the protocols axios supports?

[jasonsaayman]

If you want to put in a pull request and tag me we can fix this before that release, there is quite a lot of people that have this problem or run into it.

[fregante]

I've run into this today. Axios does not handle invalid URLs at all, resulting in such errors as:

await axios.get('httdps://example.com')
// Error: Cannot read property 'replace' of null

await axios.getUri('httdps://example.com')
// TypeError: Cannot use 'in' operator to search for 'validateStatus' in httdps://example.com

And actually I'm surprised that the supposedly-equivalent getUri options parser handles it completely differently, which kind of defeats the purpose of exposing it publicly.

[fregante]

Why close it without comment?

[nagilson]

This was said to be closed in another PR. Though I think doing something like axios.get('') in newer versions of axios still seems to result in funky behavior, for me at least.