node-miniget
node-miniget copied to clipboard
validate http libs are present
i am really thinking about renaming this https://github.com/fent/node-miniget/blob/master/src/index.ts#L165
or adding a check after https://github.com/fent/node-miniget/blob/master/src/index.ts#L11
just to make sure that people don't screw up when replacing them e.g. when using in a browser
like placing a check for parsed.protocol in httpLibs
?
instead of doing
httpLib = httpLibs[String(parsed.protocol)];
if (!httpLib) {
stream.emit('error', new Miniget.MinigetError(`Invalid URL: ${url}`));
return;
}
we could do sth like
httpLib = httpLibs[String(parsed.protocol)];
if (!httpLibs.hasOwnProperty(parsed.protocol)) {
stream.emit('error', new Miniget.MinigetError(`Invalid URL protocol: ${url}`));
return;
} else if (!httpLib) {
stream.emit('error', new Miniget.MinigetError(`unable to access http(s) library`));
return;
}
can you give an example of when the above would give the 2nd error?
from what i understood an invalid polyfill would result in the require
in this line https://github.com/fent/node-miniget/blob/master/src/index.ts#L9 sth. like null or undefined
the usual !httpLib
does not differentiate between httpLibs['https:']
being defined or being null
means a !httpLib
only tells us that it is not available atm (but not why)
a hasOwnProperty
would also tell us if it was ever assigned - even if undefined
short example:
ah i see what you mean now, this results in better error reporting. i support this