tomas / needle

Nimble, streamable HTTP client for Node.js. With proxy, iconv, cookie, deflate & multipart support.
https://www.npmjs.com/package/needle
MIT License
1.62k stars 235 forks source link

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

Open hixus opened 2 months ago

hixus commented 2 months ago

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.