Skip to content

Commit

Permalink
Several fixes (#1872)
Browse files Browse the repository at this point in the history
  • Loading branch information
ardatan authored Dec 10, 2024
1 parent 24ed1fb commit 7fb47d8
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 3 deletions.
7 changes: 7 additions & 0 deletions .changeset/chilly-beans-destroy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@whatwg-node/node-fetch': patch
---

Fix the error thrown \`ENOTFOUND\` when a parsed URL with IPV6 hostname is given

Instead of using the parsed URL passed to the `fetch` function, let `node:http` parse it again. This way, the IPV6 hostname is correctly resolved.
11 changes: 11 additions & 0 deletions .changeset/good-pans-remember.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'@whatwg-node/node-fetch': patch
---

`url.searchParams` parameter should reflect the changes in `toString()`

```ts
const url = new URL('http://example.com/?a=b');
url.searchParams.set('a', 'c');
console.log(url.toString()); // http://example.com/?a=c
```
33 changes: 33 additions & 0 deletions .changeset/shy-hotels-roll.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
'@whatwg-node/server': patch
---

Wait for remaining promises during `asyncDispose` correctly

The `asyncDispose` function should wait for all remaining promises to resolve before returning. This ensures that the server is fully disposed of before the function returns.

```ts
import { createServerAdapter } from '@whatwg-node/server';

const deferred = Promise.withResolvers();

const adapter = createServerAdapter((req, ctx) => {
ctx.waitUntil(deferred.promise);
return new Response('Hello, world!');
});

const res = await adapter.fetch('http://example.com');
console.assert(res.status === 200);
console.assert(await res.text() === 'Hello, world!');

let disposed = false;
adapter[Symbol.asyncDispose]().then(() => {
disposed = true;
});

console.assert(!disposed);

deferred.resolve();

console.assert(disposed);
```
7 changes: 7 additions & 0 deletions .changeset/sweet-mayflies-decide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@whatwg-node/node-fetch': patch
---

Fix IPV6 parsing in \`URL\`;

`new URL('http://[::1]')` should parse the host as \`[::1]\` not \`::1\`.
13 changes: 13 additions & 0 deletions packages/node-fetch/src/URL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import { PonyfillURLSearchParams } from './URLSearchParams.js';

FastUrl.queryString = FastQuerystring;

const IPV6_REGEX =
/^(?:(?:(?:[0-9A-Fa-f]{1,4}:){6}(?:[0-9A-Fa-f]{1,4}:[0-9A-Fa-f]{1,4}|(?:(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\.){3}(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]))|::(?:[0-9A-Fa-f]{1,4}:){5}(?:[0-9A-Fa-f]{1,4}:[0-9A-Fa-f]{1,4}|(?:(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\.){3}(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]))|(?:[0-9A-Fa-f]{1,4})?::(?:[0-9A-Fa-f]{1,4}:){4}(?:[0-9A-Fa-f]{1,4}:[0-9A-Fa-f]{1,4}|(?:(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\.){3}(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]))|(?:(?:[0-9A-Fa-f]{1,4}:){0,1}[0-9A-Fa-f]{1,4})?::(?:[0-9A-Fa-f]{1,4}:){3}(?:[0-9A-Fa-f]{1,4}:[0-9A-Fa-f]{1,4}|(?:(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\.){3}(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]))|(?:(?:[0-9A-Fa-f]{1,4}:){0,2}[0-9A-Fa-f]{1,4})?::(?:[0-9A-Fa-f]{1,4}:){2}(?:[0-9A-Fa-f]{1,4}:[0-9A-Fa-f]{1,4}|(?:(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\.){3}(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]))|(?:(?:[0-9A-Fa-f]{1,4}:){0,3}[0-9A-Fa-f]{1,4})?::[0-9A-Fa-f]{1,4}:(?:[0-9A-Fa-f]{1,4}:[0-9A-Fa-f]{1,4}|(?:(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\.){3}(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]))|(?:(?:[0-9A-Fa-f]{1,4}:){0,4}[0-9A-Fa-f]{1,4})?::(?:[0-9A-Fa-f]{1,4}:[0-9A-Fa-f]{1,4}|(?:(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])\.){3}(?:0?0?[0-9]|0?[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]))|(?:(?:[0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4})?::[0-9A-Fa-f]{1,4}|(?:(?:[0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4})?::)(?:\/(?:0?0?[0-9]|0?[1-9][0-9]|1[01][0-9]|12[0-8]))?)$/;

export class PonyfillURL extends FastUrl implements URL {
constructor(url: string, base?: string | URL) {
super();
Expand All @@ -16,6 +19,13 @@ export class PonyfillURL extends FastUrl implements URL {
return;
}
this.parse(url, false);
// `fast-url-parser` cannot handle ipv6 hosts correctly
if (
(url.startsWith('http://[') || url.startsWith('https://[')) &&
IPV6_REGEX.test(this.hostname)
) {
this.hostname = `[${this.hostname}]`;
}
if (base) {
const baseParsed = typeof base === 'string' ? new PonyfillURL(base) : base;
this.protocol ||= baseParsed.protocol;
Expand Down Expand Up @@ -55,6 +65,9 @@ export class PonyfillURL extends FastUrl implements URL {
}

toString(): string {
if (this._searchParams) {
this.search = this._searchParams.toString();
}
return this.format();
}

Expand Down
2 changes: 1 addition & 1 deletion packages/node-fetch/src/declarations.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ declare module '@kamilkisiela/fast-url-parser' {
path: string;
pathname: string;
protocol: string;
readonly search: string;
search: string;
slashes: boolean;
port: string;
query: string | any;
Expand Down
3 changes: 2 additions & 1 deletion packages/node-fetch/src/fetchNodeHttp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ export function fetchNodeHttp<TResponseJSON = any, TRequestJSON = any>(

let nodeRequest: ReturnType<typeof requestFn>;

if (fetchRequest.parsedUrl) {
// Skip using parsed URL if it's an IPv6 address (starting with brackets)
if (fetchRequest.parsedUrl && !fetchRequest.parsedUrl.hostname?.startsWith('[')) {
nodeRequest = requestFn({
auth: fetchRequest.parsedUrl.username
? `${fetchRequest.parsedUrl.username}:${fetchRequest.parsedUrl.password}`
Expand Down
12 changes: 12 additions & 0 deletions packages/node-fetch/tests/URL.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,16 @@ describe('URL', () => {
expect(url.searchParams.get('baz')).toBeNull();
expect(url.searchParams.getAll('baz')).toEqual([]);
});
it('should update search params', () => {
const url = new PonyfillURL('https://example.com/?foo=bar&foo=baz&bar=qux');
url.searchParams.set('foo', 'qux');
url.searchParams.delete('baz');
expect(url.toString()).toBe('https://example.com/?foo=qux&bar=qux');
});
it('parses ipv6 hosts', () => {
const url = new PonyfillURL('http://[::1]:8000');
expect(url.host).toBe('[::1]:8000');
expect(url.hostname).toBe('[::1]');
expect(url.port).toBe('8000');
});
});
2 changes: 2 additions & 0 deletions packages/server/src/createServerAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ function createServerAdapter<
}

function waitUntil(promiseLike: PromiseLike<unknown>) {
// Ensure that the disposable stack is created
ensureDisposableStack();
waitUntilPromises.add(promiseLike);
promiseLike.then(
() => {
Expand Down
14 changes: 13 additions & 1 deletion packages/server/test/node.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ describe('Node Specific Cases', () => {

// TODO: Flakey on native fetch
if (!process.env.LEAK_TEST || fetchImplName.toLowerCase() !== 'native') {
it.only('handles Request.signal inside adapter correctly', async () => {
it('handles Request.signal inside adapter correctly', async () => {
const abortListener = jest.fn();
const abortDeferred = createDeferredPromise<void>();
const adapterResponseDeferred = createDeferredPromise<Response>();
Expand Down Expand Up @@ -374,6 +374,18 @@ describe('Node Specific Cases', () => {
await setTimeout(100);
expect(disposedThen).toHaveBeenCalled();
});

it('handles ipv6 addresses correctly', async () => {
await using serverAdapter = createServerAdapter(() => {
return new Response('Hello world!', { status: 200 });
});
await testServer.addOnceHandler(serverAdapter);
const port = new URL(testServer.url).port;
const ipv6Url = new URL(`http://[::1]:${port}/`);
const response = await fetch(ipv6Url);
expect(response.status).toBe(200);
await expect(response.text()).resolves.toBe('Hello world!');
});
});
},
);
Expand Down

0 comments on commit 7fb47d8

Please sign in to comment.