Skip to content

Commit ace0812

Browse files
committed
module: prefer async/await in https imports
1 parent aff8b87 commit ace0812

File tree

2 files changed

+81
-108
lines changed

2 files changed

+81
-108
lines changed
Lines changed: 70 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,25 @@
11
'use strict';
22
const {
3-
ArrayPrototypePush,
4-
Promise,
3+
ObjectPrototypeHasOwnProperty,
54
PromisePrototypeThen,
6-
PromiseResolve,
75
SafeMap,
86
StringPrototypeEndsWith,
97
StringPrototypeSlice,
108
StringPrototypeStartsWith,
119
} = primordials;
1210
const {
13-
Buffer: {
14-
concat: BufferConcat
15-
}
11+
Buffer: { concat: BufferConcat },
1612
} = require('buffer');
1713
const {
1814
ERR_NETWORK_IMPORT_DISALLOWED,
1915
ERR_NETWORK_IMPORT_BAD_RESPONSE,
16+
ERR_MODULE_NOT_FOUND,
2017
} = require('internal/errors').codes;
2118
const { URL } = require('internal/url');
2219
const net = require('net');
2320
const path = require('path');
24-
21+
const { once } = require('events');
22+
const { compose } = require('stream');
2523
/**
2624
* @typedef CacheEntry
2725
* @property {Promise<string> | string} resolvedHREF
@@ -33,6 +31,9 @@ const path = require('path');
3331
* Only for GET requests, other requests would need new Map
3432
* HTTP cache semantics keep diff caches
3533
*
34+
* It caches either the promise or the cache entry since import.meta.url needs
35+
* the value synchronously for the response location after all redirects.
36+
*
3637
* Maps HREF to pending cache entry
3738
* @type {Map<string, Promise<CacheEntry> | CacheEntry>}
3839
*/
@@ -48,23 +49,23 @@ let HTTPSAgent;
4849
function HTTPSGet(url, opts) {
4950
const https = require('https'); // [1]
5051
HTTPSAgent ??= new https.Agent({ // [2]
51-
keepAlive: true
52+
keepAlive: true,
5253
});
5354
return https.get(url, {
5455
agent: HTTPSAgent,
55-
...opts
56+
...opts,
5657
});
5758
}
5859

5960
let HTTPAgent;
6061
function HTTPGet(url, opts) {
6162
const http = require('http'); // [1]
6263
HTTPAgent ??= new http.Agent({ // [2]
63-
keepAlive: true
64+
keepAlive: true,
6465
});
6566
return http.get(url, {
6667
agent: HTTPAgent,
67-
...opts
68+
...opts,
6869
});
6970
}
7071

@@ -99,118 +100,79 @@ function fetchWithRedirects(parsed) {
99100
return existing;
100101
}
101102
const handler = parsed.protocol === 'http:' ? HTTPGet : HTTPSGet;
102-
const result = new Promise((fulfill, reject) => {
103+
const result = (async () => {
103104
const req = handler(parsed, {
104-
headers: {
105-
Accept: '*/*'
106-
}
107-
})
108-
.on('error', reject)
109-
.on('response', (res) => {
110-
function dispose() {
111-
req.destroy();
112-
res.destroy();
113-
}
114-
if (res.statusCode >= 300 && res.statusCode <= 303) {
115-
if (res.headers.location) {
116-
dispose();
117-
try {
118-
const location = new URL(res.headers.location, parsed);
119-
if (location.protocol !== 'http:' &&
120-
location.protocol !== 'https:') {
121-
reject(new ERR_NETWORK_IMPORT_DISALLOWED(
122-
res.headers.location,
123-
parsed.href,
124-
'cannot redirect to non-network location'));
125-
return;
126-
}
127-
return PromisePrototypeThen(
128-
PromiseResolve(fetchWithRedirects(location)),
129-
(entry) => {
130-
cacheForGET.set(parsed.href, entry);
131-
fulfill(entry);
132-
});
133-
} catch (e) {
134-
dispose();
135-
reject(e);
136-
}
105+
headers: { Accept: '*/*' },
106+
});
107+
// Note that `once` is used here to handle `error` and that it hits the
108+
// `finally` on network error/timeout.
109+
const { 0: res } = await once(req, 'response');
110+
try {
111+
const isRedirect = res.statusCode >= 300 && res.statusCode <= 303;
112+
const hasLocation = ObjectPrototypeHasOwnProperty(res.headers, 'location');
113+
if (isRedirect && hasLocation) {
114+
const location = new URL(res.headers.location, parsed);
115+
if (location.protocol !== 'http:' && location.protocol !== 'https:') {
116+
throw new ERR_NETWORK_IMPORT_DISALLOWED(
117+
res.headers.location,
118+
parsed.href,
119+
'cannot redirect to non-network location'
120+
);
137121
}
122+
const entry = await fetchWithRedirects(location);
123+
cacheForGET.set(parsed.href, entry);
124+
return entry;
125+
}
126+
if (res.statusCode === 404) {
127+
const err = new ERR_MODULE_NOT_FOUND(parsed.href, null);
128+
err.message = `Cannot find module '${parsed.href}', HTTP 404`;
129+
throw err;
138130
}
139131
if (res.statusCode > 303 || res.statusCode < 200) {
140-
dispose();
141-
reject(
142-
new ERR_NETWORK_IMPORT_BAD_RESPONSE(
143-
parsed.href,
144-
'HTTP response returned status code of ' + res.statusCode));
145-
return;
132+
throw new ERR_NETWORK_IMPORT_DISALLOWED(
133+
res.headers.location,
134+
parsed.href,
135+
'cannot redirect to non-network location');
146136
}
147137
const { headers } = res;
148138
const contentType = headers['content-type'];
149139
if (!contentType) {
150-
dispose();
151-
reject(new ERR_NETWORK_IMPORT_BAD_RESPONSE(
140+
throw new ERR_NETWORK_IMPORT_BAD_RESPONSE(
152141
parsed.href,
153-
'the \'Content-Type\' header is required'));
154-
return;
142+
"the 'Content-Type' header is required"
143+
);
155144
}
156145
/**
157146
* @type {CacheEntry}
158147
*/
159148
const entry = {
160149
resolvedHREF: parsed.href,
161150
headers: {
162-
'content-type': res.headers['content-type']
151+
'content-type': res.headers['content-type'],
163152
},
164-
body: new Promise((f, r) => {
165-
const buffers = [];
166-
let size = 0;
153+
body: (async () => {
167154
let bodyStream = res;
168-
let onError;
169155
if (res.headers['content-encoding'] === 'br') {
170-
bodyStream = createBrotliDecompress();
171-
onError = function onError(error) {
172-
bodyStream.close();
173-
dispose();
174-
reject(error);
175-
r(error);
176-
};
177-
res.on('error', onError);
178-
res.pipe(bodyStream);
179-
} else if (res.headers['content-encoding'] === 'gzip' ||
180-
res.headers['content-encoding'] === 'deflate') {
181-
bodyStream = createUnzip();
182-
onError = function onError(error) {
183-
bodyStream.close();
184-
dispose();
185-
reject(error);
186-
r(error);
187-
};
188-
res.on('error', onError);
189-
res.pipe(bodyStream);
190-
} else {
191-
onError = function onError(error) {
192-
dispose();
193-
reject(error);
194-
r(error);
195-
};
156+
bodyStream = compose(res, createBrotliDecompress());
157+
} else if (
158+
res.headers['content-encoding'] === 'gzip' ||
159+
res.headers['content-encoding'] === 'deflate'
160+
) {
161+
bodyStream = compose(res, createUnzip());
196162
}
197-
bodyStream.on('error', onError);
198-
bodyStream.on('data', (d) => {
199-
ArrayPrototypePush(buffers, d);
200-
size += d.length;
201-
});
202-
bodyStream.on('end', () => {
203-
const body = entry.body = /** @type {Buffer} */(
204-
BufferConcat(buffers, size)
205-
);
206-
f(body);
207-
});
208-
}),
163+
const buffers = await bodyStream.toArray();
164+
const body = BufferConcat(buffers);
165+
entry.body = body;
166+
return body;
167+
})(),
209168
};
210169
cacheForGET.set(parsed.href, entry);
211-
fulfill(entry);
212-
});
213-
});
170+
await entry.body;
171+
return entry;
172+
} finally {
173+
req.destroy();
174+
}
175+
})();
214176
cacheForGET.set(parsed.href, result);
215177
return result;
216178
}
@@ -227,8 +189,10 @@ allowList.addRange('127.0.0.1', '127.255.255.255');
227189
*/
228190
async function isLocalAddress(hostname) {
229191
try {
230-
if (StringPrototypeStartsWith(hostname, '[') &&
231-
StringPrototypeEndsWith(hostname, ']')) {
192+
if (
193+
StringPrototypeStartsWith(hostname, '[') &&
194+
StringPrototypeEndsWith(hostname, ']')
195+
) {
232196
hostname = StringPrototypeSlice(hostname, 1, -1);
233197
}
234198
const addr = await dnsLookup(hostname, { verbatim: true });
@@ -265,10 +229,8 @@ function fetchModule(parsed, { parentURL }) {
265229
if (is !== true) {
266230
let parent = parentURL;
267231
const parentName = path.basename(parent.pathname);
268-
if (
269-
parentName === '[eval]' ||
270-
parentName === '[stdin]'
271-
) parent = 'command-line';
232+
if (parentName === '[eval]' || parentName === '[stdin]')
233+
parent = 'command-line';
272234
throw new ERR_NETWORK_IMPORT_DISALLOWED(
273235
href,
274236
parent,
@@ -282,5 +244,5 @@ function fetchModule(parsed, { parentURL }) {
282244
}
283245

284246
module.exports = {
285-
fetchModule: fetchModule
247+
fetchModule: fetchModule,
286248
};

test/es-module/test-http-imports.mjs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ for (const { protocol, createServer } of [
6464
const server = createServer(function(_req, res) {
6565
const url = new URL(_req.url, host);
6666
const redirect = url.searchParams.get('redirect');
67+
if (url.pathname === '/not-found') {
68+
res.writeHead(404);
69+
res.end();
70+
return;
71+
}
6772
if (redirect) {
6873
const { status, location } = JSON.parse(redirect);
6974
res.writeHead(status, {
@@ -164,6 +169,12 @@ for (const { protocol, createServer } of [
164169
import(unsupportedMIME.href),
165170
'should not be able to load unsupported MIMEs from http:'
166171
);
172+
const notFound = new URL(url.href);
173+
notFound.pathname = '/not-found';
174+
await assert.rejects(
175+
import(notFound.href),
176+
{ code: 'ERR_MODULE_NOT_FOUND' },
177+
);
167178

168179
server.close();
169180
}

0 commit comments

Comments
 (0)