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

fix: TypeError crashes program in case of invalid url in redirect #59

Closed
wants to merge 2 commits into from

Conversation

MohGanji
Copy link

Description

Calling fetch() on a server that responds with an invalid URL breaks the execution of the program with the following unhandled error:

internal/url.js:259
  throw new ERR_INVALID_URL(input);
  ^

TypeError [ERR_INVALID_URL]: Invalid URL: :some:weird%invalid%url%

Steps to reproduce:

  1. To simulate the environment that the bug appears in, set up a local server redirecting to an invalid URL
const http = require('http');
const server = http.createServer((req, res, next) => {
	res.writeHead(301, {'Location' : 'http://www.stackoverflow.com%'});
	res.end();
});
server.listen(process.env.PORT || 3000);
  1. Then running this code throws error and crashes the program despite fetch() being wrapped in a try/catch block.
let fetch = require('minipass-fetch')

(async function {
	try{
		var res = await fetch(
			'http://localhost:3000',
		//	{redirect: 'manual'}
		)
		console.log('val: ', res)
	} catch (err) {
		console.log('err: ', err)
	}
})()

References

This behavior is inconsistent with current node-fetch. Similar issue reported #1386 and fixed #1387 in node-fetch.

@wraithgar
Copy link
Member

@MohGanji can you rebase this pr against main? The CI has been fixed so we should be able to see if tests pass here. Your tests will also need to be tweaked to use async/await like the rest of the tests were so that linting passes.

@wraithgar
Copy link
Member

#100

@wraithgar wraithgar closed this Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants