tomas / tomas/needle

Invalid protocol causes uncatchable error in node:http(s)

Open
#436 0 comments 0 reactions 0 assignees View on GitHub

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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.