Skip to content

Commit

Permalink
fix: Don't mutate passed URL object (#470)
Browse files Browse the repository at this point in the history
Before this change, a passed URL object would be mutated if query
parameters were appended via the `data` option.

Now we clone the object instead, so the original is not affected.

---

Background:

I am passing `URL` objects to the `request` method, and then adding
query parameters via the `data` option. Example:

```js
import { request } from "urllib";

const url = new URL("http://example.com");

await request(url, { data: { param1: "value1" } });
```

I found that if I then make another request to the same URL but with
different parameters:

```js
await request(url, { data: { param2: "value2" } });
```

...a request is actually made to
`http://example.com?param1=value1&param2=value2`.

My URL object is being mutated by the urllib library. I see this as a
bug.
  • Loading branch information
tremby authored Sep 21, 2023
1 parent 8abf3cd commit 5b8867f
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/HttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ export class HttpClient extends EventEmitter {
// url maybe url.parse(url) object in urllib2
requestUrl = new URL(urlFormat(url));
} else {
requestUrl = url;
// or even if not, we clone to avoid mutating it
requestUrl = new URL(url.toString());
}
}

Expand Down
9 changes: 9 additions & 0 deletions test/options.data.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ describe('options.data.test.ts', () => {
assert.equal(url.searchParams.get('data'), '哈哈');
});

it('should not mutate a passed URL object when setting query string', async () => {
const url = new URL(_url);
assert.equal(url.searchParams.get('param1'), null);
await urllib.request(url, {
data: { param1: 'val1' },
});
assert.equal(url.searchParams.get('param1'), null);
});

it('should GET with data work on nestedQuerystring=true', async () => {
const response = await urllib.request(_url, {
method: 'GET',
Expand Down

0 comments on commit 5b8867f

Please sign in to comment.