Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate http libs are present #63

Open
TimeForANinja opened this issue Aug 4, 2021 · 5 comments
Open

validate http libs are present #63

TimeForANinja opened this issue Aug 4, 2021 · 5 comments
Labels

Comments

@TimeForANinja
Copy link
Collaborator

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

@fent
Copy link
Owner

fent commented Jan 8, 2022

like placing a check for parsed.protocol in httpLibs?

@TimeForANinja
Copy link
Collaborator Author

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;
}

@fent
Copy link
Owner

fent commented Jan 16, 2022

can you give an example of when the above would give the 2nd error?

@TimeForANinja
Copy link
Collaborator Author

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:
image

@fent
Copy link
Owner

fent commented Jan 25, 2022

ah i see what you mean now, this results in better error reporting. i support this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants