Invalid protocol causes uncatchable error in node:http(s)
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 1.6k
- Forks
- 237
- PR merge metrics
- No merged PRs in 30d
Description
Hi! 👋
Firstly, thanks for your work on this project! 🙂
Today I used patch-package to patch needle@3.3.1 for the project I'm working on.
Mobile deeplinks can redirect to non http(s) and needle only support node http/https modules. Random mobile app protocol will cause TypeError [ERR_INVALID_PROTOCOL]: Protocol "farcaster:" not supported. Expected "http:" which is not possible to catch. This will make our link preview indexer to crash.
I added two solutions but not sure if neither is completely correct.
First one checks if protocol in should_follow and other after should_follow check. Results bit different errors but ideally would like to log original url and the redirect url which is invalid.
Here is the diff that solved my problem:
diff --git a/node_modules/needle/lib/needle.js b/node_modules/needle/lib/needle.js
index e153b92..4ceb8a9 100644
--- a/node_modules/needle/lib/needle.js
+++ b/node_modules/needle/lib/needle.js
@@ -420,6 +420,9 @@ Needle.prototype.get_request_opts = function(method, uri, config) {
Needle.prototype.should_follow = function(location, config, original) {
if (!location) return false;
+ // http and https are the only supported protocols for redirects
+ if (location.indexOf('http') !== 0) return false
+
// returns true if location contains matching property (host or protocol)
function matches(property) {
var property = original[property];
@@ -526,6 +529,10 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data,
// if redirect code is found, determine if we should follow it according to the given options.
if (redirect_codes.indexOf(resp.statusCode) !== -1 && self.should_follow(headers.location, config, uri)) {
+ if (headers.location.indexOf('http') !== 0) {
+ return done(new Error('Unsupported protocol in location: ' + headers.location));
+ }
+
// clear timer before following redirects to prevent unexpected setTimeout consequence
clearTimeout(timer);
This issue body was partially generated by patch-package.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in node_modules/needle/lib/needle.js at Needle.prototype.should_follow and Needle.prototype.send_request, then trace how redirect locations are handled. Compare the two proposed guard locations and verify the behavior for a redirect to a farcaster: URL. Done means the invalid protocol produces a catchable error instead of an uncatchable TypeError, with redirect context preserved if possible.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100