Skip to content

Commit

Permalink
Move request.url to getter
Browse files Browse the repository at this point in the history
  • Loading branch information
@hueniverse
hueniverse committed Oct 31, 2019
1 parent bdfd254 commit 6b67d24
Show file tree
Hide file tree
Showing 4 changed files with 219 additions and 32 deletions.
105 changes: 79 additions & 26 deletions lib/request.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

const Querystring = require('querystring');
const Url = require('url');

const Boom = require('@hapi/boom');
Expand Down Expand Up @@ -34,6 +35,7 @@ exports = module.exports = internals.Request = class {
this._route = this._core.router.specials.notFound.route; // Used prior to routing (only settings are used, not the handler)
this._serverTimeoutId = null;
this._states = {};
this._url = null;
this._urlError = null;

this.app = options.app ? Object.assign({}, options.app) : {}; // Place for application-specific state without conflicts with hapi, should not be used by plugins (shallow cloned)
Expand All @@ -57,7 +59,6 @@ exports = module.exports = internals.Request = class {
this.query = null;
this.server = server;
this.state = null;
this.url = null;

this.auth = {
isAuthenticated: false,
Expand Down Expand Up @@ -101,6 +102,19 @@ exports = module.exports = internals.Request = class {
return this._events;
}

get url() {

if (this._urlError) {
return null;
}

if (this._url) {
return this._url;
}

return this._parseUrl(this.raw.req.url, this._core.settings.router);
}

_initializeUrl() {

try {
Expand All @@ -125,53 +139,92 @@ exports = module.exports = internals.Request = class {
Hoek.assert(typeof url === 'string', 'Url must be a string or URL object');

this._setUrl(url, stripTrailingSlash);
this._urlError = null;
}

_setUrl(url, stripTrailingSlash) {
_setUrl(source, stripTrailingSlash) {

const base = url[0] === '/' ? `${this._core.info.protocol}://${this.info.host || `${this._core.info.host}:${this._core.info.port}`}` : '';
const url = this._parseUrl(source, { stripTrailingSlash, _fast: true });
this.query = this._parseQuery(url.searchParams);
this.path = url.pathname;
}

url = new Url.URL(base + url);
_parseUrl(source, options) {

// Apply path modifications
if (source[0] === '/') {

let path = this._core.router.normalize(url.pathname); // pathname excludes query
// Relative URL

if (stripTrailingSlash &&
path.length > 1 &&
path[path.length - 1] === '/') {
if (options._fast) {
const url = {
pathname: source,
searchParams: ''
};

path = path.slice(0, -1);
const q = source.indexOf('?');
if (q !== -1) {
url.pathname = source.slice(0, q);
const h = source.indexOf('#', q + 1);

This comment has been minimized.

Sign in to view

Copy link
@kanongil

kanongil Nov 7, 2019 •
edited
Loading

Contributor

This logic misinterprets a ? char in a hash, eg. /a#b?c would incorrectly set pathname = '/a#b' and query = 'c'.

This comment has been minimized.

Sign in to view

Copy link
@hueniverse

hueniverse Nov 7, 2019

Author Contributor

Good point
const query = h === -1 ? source.slice(q + 1) : source.slice(q + 1, h);
url.searchParams = Querystring.parse(query);
}
else {
const h = source.indexOf('#');
url.pathname = h === -1 ? source : source.slice(0, h);
}

this._normalizePath(url, options);
return url;
}

this._url = new Url.URL(`${this._core.info.protocol}://${this.info.host || `${this._core.info.host}:${this._core.info.port}`}${source}`);
}
else {

url.pathname = path;
// Absolute URI (proxied)

// Parse query (must be done before this.url is set in case query parsing throws)
this._url = new Url.URL(source);
this.info.hostname = this._url.hostname;
this.info.host = this._url.host;
}

this.query = this._parseQuery(url.searchParams);
this._normalizePath(this._url, options);
this._urlError = null;

return this._url;
}

_normalizePath(url, options) {

let path = this._core.router.normalize(url.pathname);

// Store request properties
if (options.stripTrailingSlash &&
path.length > 1 &&
path[path.length - 1] === '/') {

this.url = url;
this.path = path;
path = path.slice(0, -1);
}

this.info.hostname = url.hostname;
this.info.host = url.host;
url.pathname = path;
}

_parseQuery(searchParams) {

let query = Object.create(null);

// Flatten map

let query = Object.create(null);
for (let [key, value] of searchParams) {
const entry = query[key];
if (entry !== undefined) {
value = [].concat(entry, value);
}
if (searchParams instanceof Url.URLSearchParams) {
for (let [key, value] of searchParams) {
const entry = query[key];
if (entry !== undefined) {
value = [].concat(entry, value);
}

query[key] = value;
query[key] = value;
}
}
else {
query = Object.assign(query, searchParams);
}

// Custom parser
Expand Down
2 changes: 1 addition & 1 deletion lib/transmit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ internals.end = function (env, event, err) {
Response.drain(stream);
}

err = err || new Boom(`Request ${event}`, { statusCode: request.route.settings.response.disconnectStatusCode });
err = err || new Boom.Boom(`Request ${event}`, { statusCode: request.route.settings.response.disconnectStatusCode });
const error = internals.error(request, Boom.boomify(err));
request._setResponse(error);

Expand Down
10 changes: 5 additions & 5 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
"dependencies": {
"@hapi/accept": "3.x.x",
"@hapi/ammo": "3.x.x",
"@hapi/boom": "7.x.x",
"@hapi/boom": "8.x.x",
"@hapi/bounce": "1.x.x",
"@hapi/call": "5.x.x",
"@hapi/call": "6.x.x",
"@hapi/catbox": "10.x.x",
"@hapi/catbox-memory": "4.x.x",
"@hapi/heavy": "6.x.x",
"@hapi/hoek": "8.x.x",
"@hapi/hoek": "^8.5.0",
"@hapi/joi": "16.x.x",
"@hapi/mimos": "4.x.x",
"@hapi/podium": "3.x.x",
Expand All @@ -35,10 +35,10 @@
"@hapi/topo": "3.x.x"
},
"devDependencies": {
"@hapi/code": "6.x.x",
"@hapi/code": "7.x.x",
"@hapi/inert": "5.x.x",
"@hapi/joi-legacy-test": "npm:@hapi/joi@15.x.x",
"@hapi/lab": "20.x.x",
"@hapi/lab": "21.x.x",
"@hapi/wreck": "15.x.x",
"@hapi/vision": "5.x.x",
"handlebars": "4.x.x"
Expand Down
134 changes: 134 additions & 0 deletions test/request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1271,6 +1271,140 @@ describe('Request', () => {
expect(res.statusCode).to.equal(200);
expect(res.result).to.equal({ p: '/path', path: '//path', hostname: server.info.host.toLowerCase() });
});

it('handles escaped path segments', async () => {

const server = Hapi.server();
server.route({ path: '/%2F/%2F', method: 'GET', handler: (request) => request.path });

const tests = [
['/', 404],
['////', 404],
['/%2F/%2F', 200, '/%2F/%2F'],
['/%2F/%2F#x', 200, '/%2F/%2F'],
['/%2F/%2F?a=1#x', 200, '/%2F/%2F']
];

for (const [uri, code, result] of tests) {
const res = await server.inject(uri);
expect(res.statusCode).to.equal(code);

if (code < 400) {
expect(res.result).to.equal(result);
}
}
});

it('handles fragments (no query)', async () => {

const server = Hapi.server();
server.route({ method: 'GET', path: '/{p*}', handler: (request) => request.path });

await server.start();

const options = {
hostname: 'localhost',
port: server.info.port,
path: '/path#ignore',
method: 'GET'
};

const team = new Teamwork();
const req = Http.request(options, (res) => team.attend(res));
req.end();

const res = await team.work;
const payload = await Wreck.read(res);
expect(payload.toString()).to.equal('/path');

await server.stop();
});

it('handles fragments (with query)', async () => {

const server = Hapi.server();
server.route({ method: 'GET', path: '/{p*}', handler: (request) => request.query.a });

await server.start();

const options = {
hostname: 'localhost',
port: server.info.port,
path: '/path?a=1#ignore',
method: 'GET'
};

const team = new Teamwork();
const req = Http.request(options, (res) => team.attend(res));
req.end();

const res = await team.work;
const payload = await Wreck.read(res);
expect(payload.toString()).to.equal('1');

await server.stop();
});

it('handles absolute URL (proxy)', async () => {

const server = Hapi.server();
server.route({ method: 'GET', path: '/{p*}', handler: (request) => request.query.a.join() });

await server.start();

const options = {
hostname: 'localhost',
port: server.info.port,
path: 'http://example.com/path?a=1&a=2#ignore',
method: 'GET'
};

const team = new Teamwork();
const req = Http.request(options, (res) => team.attend(res));
req.end();

const res = await team.work;
const payload = await Wreck.read(res);
expect(payload.toString()).to.equal('1,2');

await server.stop();
});
});

describe('url', () => {

it('generates URL object lazily', async () => {

const server = Hapi.server();

const handler = (request) => {

expect(request._url).to.not.exist();
return request.url.pathname;
};

server.route({ path: '/test', method: 'GET', handler });
const res = await server.inject('/test?a=1');
expect(res.statusCode).to.equal(200);
expect(res.result).to.equal('/test');
});

it('generates URL object lazily (no host header)', async () => {

const server = Hapi.server();

const handler = (request) => {

delete request.info.host;
expect(request._url).to.not.exist();
return request.url.pathname;
};

server.route({ path: '/test', method: 'GET', handler });
const res = await server.inject('/test?a=1');
expect(res.statusCode).to.equal(200);
expect(res.result).to.equal('/test');
});
});

describe('_tap()', () => {
Expand Down

0 comments on commit 6b67d24

Please sign in to comment.