microsoft / typed-rest-client

Node Rest and Http Clients with typings for use with TypeScript
Other
675 stars 118 forks source link

Redirects using relative URLs are not resolved using base URL #201

Closed DonMcNamara closed 4 years ago

DonMcNamara commented 4 years ago

Environment

Node version: v12.14.1 Npm version: 6.13.4 OS and version: Ubuntu 18.04.1 typed-rest-client version: 1.7.2

Issue Description

Redirect URLs are being resolved incorrectly when the redirect location is a relative URL.

Expected behaviour

Redirect locations that are relative should be resolved using the URL of the current request as the base URL.

Actual behaviour

The URL is parsed as an absolute URL. This results in redirecting back to localhost.

Steps to reproduce

import * as httpm from "typed-rest-client/HttpClient";
import { IRequestOptions } from "typed-rest-client/Interfaces";

const userAgent = "";
const options: IRequestOptions = { allowRedirectDowngrade: true };
const client = new httpm.HttpClient(userAgent, [], options);

// This URL will result in a redirect to a relative URL.
const url = "https://postman-echo.com/cookies/set?foo1=bar1&foo2=bar2";
const headers = {};

// This fails with: `Error: connect ECONNREFUSED 127.0.0.1:80`
const result = client.request("GET", url, "", headers);

Logs

(node:9592) UnhandledPromiseRejectionWarning: Error: connect ECONNREFUSED 127.0.0.1:80
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1134:16)

Relevant curl output:

curl -v https://postman-echo.com/cookies/set\?foo1\=bar1\&foo2\=bar2
< HTTP/1.1 302 Found
< Content-Type: text/plain; charset=utf-8
< Date: Tue, 03 Mar 2020 23:19:51 GMT
< Location: /cookies
DonMcNamara commented 4 years ago

I think this is a relatively easy fix. I think we just need to use the url constructor that takes a base argument: https://nodejs.org/api/url.html#url_constructor_new_url_input_base here: https://github.com/microsoft/typed-rest-client/blob/master/lib/HttpClient.ts#L279-L289

I can follow up with a PR soon, but if anyone beats me to it, I won't mind :wink:

stephenmichaelf commented 4 years ago

Thanks for opening this @DonMcNamara ! We happily accept PRs :)

github-actions[bot] commented 4 years ago

This issue has had no activity in 90 days. Please comment if it is not actually stale

rogerzxu commented 3 years ago

I'm also running into this issue, will submit a PR. Anyone know of a workaround in the meantime?

micsen commented 1 year ago

Any updates on this? We ran into the same issue still