Prevent URL pollution

[Giotino]

Checklist

• I have read the documentation.
• I have included a pull request description of my changes.
• I have included some tests.

Keeping in mind that

Properties from options will override properties in the parsed url.

URL's username ans password are overwritten only if they're not empty (or null) in options.

I'm going to add some tests.

Fixes #1235

[Giotino]

Added some tests

[Giotino]

There's no documentation about if username and password should be replaced (or merged) together or separately.

There's a test response contains got options which fails when the username and password are not replaced together.
The problem is that the URL (only if an object, not a string) from the given ExtendedGot is modified by each call and, as it's an object (reference), it's the same object for each call.

Is this an expected behavior? If not the URL should be deep-copied on each call to prevent pollution.

https://github.com/sindresorhus/got/blob/master/source/core/index.ts#L583

Is only a 1 level deep copy, url is copied as a reference that is not enough.

My suggestion is to add something like

if (is.urlInstance(options.url)) {
  options.url = new URL(options.url.toString());
}

after line 587

[szmarczak]

There's no documentation about if username and password should be replaced (or merged) together or separately.

Seperately. E.g. a user can have an empty password.

[szmarczak]

If not the URL should be deep-copied on each call to prevent pollution.

Not exactly object copy, but new URL(url).

[szmarczak]

This also needs a separate test.

[Giotino]

If not the URL should be deep-copied on each call to prevent pollution.

Not exactly object copy, but new URL(url).

Yes, I'm doing it

[Giotino]

Check this last commit

[szmarczak]

Hint: git pull origin master --rebase. So you don't need to commit the Merge branch 'master' into ....

[szmarczak]

This just needs a separate test to check against the pollution and we're good to go! :)

[Giotino]

Hint: git pull origin master --rebase. So you don't need to commit the Merge branch 'master' into ....

Let's say I need to get used to git... :)

BTW, I added a test, it's like the one which was failing, but simpler

[szmarczak]

Let's say I need to get used to git... :)

I've been there too ;)

BTW, I added a test, it's like the one which was failing, but simpler

Let me see it.

[szmarczak]

Good job! Thanks 🙌