Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove usage of deprecated url.Parse #242

Merged
merged 3 commits into from Mar 30, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
Expand Up @@ -104,7 +104,7 @@ Initiate an HTTP request.
- `baseUrl` - fully qualified uri string used as the base url. Most useful with `request.defaults`, for example when you want to do many requests to the same domain.
If `baseUrl` is `https://example.com/api/`, then requesting `/end/point?test=true` will fetch `https://example.com/api/end/point?test=true`. Any
querystring in the `baseUrl` will be overwritten with the querystring in the `uri` When `baseUrl` is given, `uri` must also be a string.
- `socketPath` - `/path/to/unix/socket` for Server.
- `socketPath` - `/path/to/unix/socket` for Server. When using `socketPath` the `uri` parameter to `request` and the shortcut functions will be ignored.
- `payload` - The request body as a string, Buffer, Readable Stream, or an object that can be serialized using `JSON.stringify()`.
- `headers` - An object containing request headers.
- `redirects` - The maximum number of redirects to follow.
Expand Down Expand Up @@ -333,7 +333,7 @@ To enable events, use `Wreck.defaults({ events: true })`. Events are available v
The request event is emitted just before *wreck* creates a request. The
handler should accept the following arguments `(uri, options)` where:

- `uri` - the result of `Url.parse(uri)`. This will provide information about
- `uri` - the result of `new URL(uri)`. This will provide information about
the resource requested. Also includes the headers and method.
- `options` - the options passed into the request function. This will include
a payload if there is one.
Expand All @@ -360,7 +360,7 @@ handler should accept the following arguments `(err, details)` where:
- `req` - the raw `ClientHttp` request object
- `res` - the raw `IncomingMessage` response object
- `start` - the time that the request was initiated
- `uri` - the result of `Url.parse(uri)`. This will provide information about
- `uri` - the result of `new URL(uri)`. This will provide information about
the resource requested. Also includes the headers and method.

This event is useful for logging all requests that go through *wreck*. The `err`
Expand Down
34 changes: 18 additions & 16 deletions lib/index.js
Expand Up @@ -57,24 +57,27 @@ internals.Client.prototype.defaults = function (options) {
};


// baseUrl needs to end in a trailing / if it contains paths that need to be preserved
internals.resolveUrl = function (baseUrl, path) {

if (!path) {
return baseUrl;
}

const parsedPath = Url.parse(path);
if (parsedPath.host) {
Hoek.assert(parsedPath.protocol, 'Invalid destination path missing protocol');
return Url.format(parsedPath);
// The URL constructor does a resolve, so need to trim the leading / from path
if (path.startsWith('/')) {
path = path.substr(1);
}

const parsedBase = Url.parse(baseUrl);
parsedBase.pathname = parsedBase.pathname + parsedPath.pathname;
parsedBase.pathname = parsedBase.pathname.replace(/[/]{2,}/g, '/');
parsedBase.search = parsedPath.search; // Always use the querystring from the path argument
const base = new Url.URL(baseUrl);
// When the pathname doesn't end in a trailing / it won't be part of the formatted url returned
if (!base.pathname.endsWith('/')) {
base.pathname += '/';
geek marked this conversation as resolved.
Show resolved Hide resolved
}

return Url.format(parsedBase);
// Will default to path if it's not a relative URL
const url = new Url.URL(path, base);
return Url.format(url);
};


Expand Down Expand Up @@ -124,22 +127,20 @@ internals.Client.prototype.request = function (method, url, options = {}) {
internals.Client.prototype._request = function (method, url, options, relay, _trace) {

const uri = {};
let parsedUri;
if (options.socketPath) {
uri.socketPath = options.socketPath;
delete options.socketPath;
parsedUri = Url.parse(url);
// The host must be empty according to https://tools.ietf.org/html/rfc2616#section-14.23
uri.host = '';
}
else {
uri.setHost = false;
parsedUri = new Url.URL(url);
const parsedUri = new Url.URL(url);
internals.applyUrlToOptions(uri, parsedUri);
}

internals.applyUrlToOptions(uri, parsedUri);

uri.method = method.toUpperCase();
uri.headers = options.headers || {};
uri.headers.host = parsedUri.host;
uri.headers.host = uri.host;
const hasContentLength = internals.findHeader('content-length', uri.headers) !== undefined;

if (options.payload && typeof options.payload === 'object' && !(options.payload instanceof Stream) && !Buffer.isBuffer(options.payload)) {
Expand Down Expand Up @@ -664,6 +665,7 @@ internals.findHeader = function (headerName, headers) {

internals.applyUrlToOptions = (options, url) => {

options.host = url.host;
options.origin = url.origin;
options.searchParams = url.searchParams;
options.protocol = url.protocol;
Expand Down
18 changes: 17 additions & 1 deletion test/index.js
Expand Up @@ -975,6 +975,14 @@ describe('request()', () => {
expect(body.toString()).to.equal(internals.payload);
server.close();
});

it('requests a POST resource with headers using post shortcut', async () => {

const server = await internals.server('echo', internals.socket);
const { payload } = await Wreck.post('/', { socketPath: internals.socket, headers: { 'user-agent': 'wreck' }, payload: internals.payload });
expect(payload.toString()).to.equal(internals.payload);
server.close();
});
});
});

Expand Down Expand Up @@ -1035,9 +1043,17 @@ describe('options.baseUrl', () => {
expect(promise.req.path).to.equal('/foo/bar');
});

it('includes the full path regardless of it begins with a slash', async () => {

const promise = Wreck.request('get', 'baz', { baseUrl: 'http://localhost:0/foo/bar' });
await expect(promise).to.reject();
expect(promise.req._headers.host).to.equal('localhost:0');
expect(promise.req.path).to.equal('/foo/bar/baz');
});

it('uses baseUrl option with a url that has a querystring', async () => {

const promise = Wreck.request('get', '/bar?test=hello', { baseUrl: 'http://localhost:0/foo' });
const promise = Wreck.request('get', '/bar?test=hello', { baseUrl: 'http://localhost:0/foo/' });
await expect(promise).to.reject();
expect(promise.req._headers.host).to.equal('localhost:0');
expect(promise.req.path).to.equal('/foo/bar?test=hello');
Expand Down