Redirect with Location partial URL doesn't work

[swordensen]

Hey guys,

I'm working on an internal tool so I can't share a test case unfortunately (since I don't know of another server that creates this behavior). However, I thought I would toss my discovery and solution towards the project in case anyone else also discovers this issue and can provide a test case.

My Problem

Basically the server was sending a redirect request back to my app with headers that looked like this instead of a full URL:

{
.
.
.
location: /foobar/
.
.
.
}

then when node-fetch would try to create the request, Node would throw the error: TypeError [ERR_INVALID_URL]: Invalid URL

My Solution

I wrote a little function that will return a full url using the protocol provided by the request agent and the host url provided in the request headers and then use that for the redirect URL instead of just the location header.

  /**
   * formats the URL by adding the host and protocol to the url string if the first character of the location url is '/'
   * @param {String} url 
   * @param {*} opts 
   * @returns {String}
   */
  function getRedirectUrl(url, opts){
    const firstCharacterIsSlash = url.slice(0,1) === '/';
    if(!firstCharacterIsSlash) return url;

    const host = opts.headers.Host;
    const protocol = opts.agent.protocol;
    return `${protocol}//${host}${url}`
  }

I can create a pull request if desired but, honestly I am unsure if this is actually the best solution since I'm not exactly an expert in this area. This just solved my problem.

[Edit] I made these updates to the node-fetch.js file

[valeriangalliat]

Yo! It seems like it's related to node-fetch, not fetch-cookie, is that right?

Even if you can't share the original source code, the best would be that you create the smallest repro possible to document the bug, for example:

const http = require('http')

const server = http.createServer((req, res) => {
  if (req.url.startsWith('/foo')) {
    res.writeHead(302, { location: '/bar' })
    res.end()
    return
  }

  res.end('hello')
})

server.listen(8000)

This server returns the kind of location header that you mention broke your app.

Then with this:

const nodeFetch = require('node-fetch')
const fetch = require('fetch-cookie')(nodeFetch)
fetch('http://localhost:8000/foo').then(res => res.text()).then(console.log, console.log)

We can test the PoC. But in my case this example works, the redirect happens properly.

Cheers!

[swordensen]

Oh awesome, I was actually struggling to create a small example since express was automagically formatting /bar => http://localhost:8000/bar

Thanks for the demo I'll see if I can reproduce with this!

[swordensen]

So I was also unable to recreate the issue with the test suite. However, I was able to do so by updating the node-fetch version to 3.2.0 .

@valeriangalliat Would that be considered out of scope?

[valeriangalliat]

That would be in scope, fetch-cookie should be compatible with node-fetch 3.2.0.

It's still unclear to me what makes you think this is fetch-cookie related and not node-fetch related though! Can you share with me the code that allowed you to repro the error?

I tried with node-fetch 3.2.0 with the following poc.mjs:

import nodeFetch from 'node-fetch'
import fetchCookie from 'fetch-cookie'

const fetch = fetchCookie(nodeFetch)
fetch('http://localhost:8000/foo').then(res => res.text()).then(console.log, console.log)

But the redirect do happen properly for me!

[swordensen]

Note this was using node-fetch 3.2.0 (totally get if this is not supported since the package.json indicates 2.6.0

Okay here was the test I was created ( I am sorry it's not perfect, I'm trying to be quick lol )

/* eslint-env mocha */
import nodeFetch from 'node-fetch'
import fetchCookie from '../node-fetch.js';
const fetch = fetchCookie(nodeFetch);
import express from 'express';

describe('partial redirect failure', () => {
  // setup
  let server;
  before('start test server', async () => {
    return new Promise((resolve, reject) => {
      server = express();
      server.get('/partialredirect', (req, res) => {
        res.redirect('/get')
      });
      server.get('/get', (req, res) => {
        res.status(200).send('successfully reached redirect point!')
      });

      server.listen(8000, err => {
        if (err) { reject(err) } else { resolve() }
      });
    })
  })

  it('should be able to redirect from a partial url', async () => {
    // fails
    const response = await fetch('http://localhost:8000/partialredirect')
  })

  after('should turn off server', () => {
    console.log(server);
    if (server) {
      server.close();
    }
  })
})

I did notice that I am importing fetch cookie from the node-fetch.js file which according to the docs should preserve cookies when redirecting. It does do that but, I do get the invalid URL error from node-fetch when using this file.

I did also try the normal index.js file like you were using and the redirect works fine just the cookies are not there lol.

I hope that brings more clarity. I'm happy to close the issue or keep drilling down to find the root problem.

[valeriangalliat]

Oh I didn't realize this was a problem with the node-fetch wrapper. I could pinpoint the issue, it's fixed with fetch-cookie@1.0.1!

Cheers!

[swordensen]

you're a legend. thanks!