Skip to content

Commit

Permalink
break(url): handle decodes correctly; trim ParsedURL properties (#165)
Browse files Browse the repository at this point in the history
* break(url): handle decodes correctly; remove output keys;

- Closes #150

* chore(url): update readme

* break(url): use named `parse` export

* fix(polka): rebuild `req.url` manually;

- includes decoded `req.path` and ENCODED `req.search` segments

* fix(polka): adjust `Request` types

* chore(url): update benchmark

* chore(url): update benchmark results

* chore(url): add "raw performance" results

* chore: linter
  • Loading branch information
lukeed authored May 23, 2021
1 parent de206c3 commit 66f9cc5
Show file tree
Hide file tree
Showing 11 changed files with 633 additions and 374 deletions.
5 changes: 4 additions & 1 deletion packages/polka/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@ export interface IOptions<T extends Request = Request> {

export type Response = ServerResponse;

export interface Request extends IncomingMessage, ParsedURL {
export interface Request extends IncomingMessage {
url: string;
method: string;
originalUrl: string;
params: Record<string, string>;
path: string;
search: string;
query: Record<string,string>;
body?: any;
_decoded?: true;
_parsedUrl: ParsedURL;
Expand Down
9 changes: 5 additions & 4 deletions packages/polka/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import http from 'http';
import Router from 'trouter';
import parser from '@polka/url';
import { parse } from '@polka/url';

function onError(err, req, res) {
let code = (res.statusCode = err.code || err.status || 500);
Expand All @@ -13,7 +13,7 @@ const mount = fn => fn instanceof Polka ? fn.attach : fn;
class Polka extends Router {
constructor(opts={}) {
super();
this.parse = parser;
this.parse = parse;
this.server = opts.server;
this.handler = this.handler.bind(this);
this.onError = opts.onError || onError; // catch-all handler
Expand Down Expand Up @@ -45,9 +45,9 @@ class Polka extends Router {
},
fns.map(mount),
(req, _, next) => {
req.url = req._parsedUrl.href;
req.path = req._parsedUrl.pathname;
next()
req.url = req.path + req._parsedUrl.search;
next();
}
);
}
Expand All @@ -66,6 +66,7 @@ class Polka extends Router {

req.params = obj.params;
req.originalUrl = req.originalUrl || req.url;
req.url = info.pathname + info.search;
req.query = info.query || {};
req.search = info.search;

Expand Down
10 changes: 5 additions & 5 deletions packages/polka/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,7 @@ if (hasNamedGroups) {
assert.is(req.path, '/comments', '~> sees correct `path` value – REPLACED PATTERN');
assert.is(req.url, '/comments?foo', '~> sees correct `url` value – REPLACED PATTERN');
assert.is(req.params.title, 'narnia', '~> receives correct `params.title` value');
assert.equal(req.query, { foo: '' }, '~> receives correct `req.query` value');
assert.equal({ ...req.query }, { foo: '' }, '~> receives correct `req.query` value');
res.end('cya~!');
}) // global
.use(/^\/songs.*/i, (req, res) => {
Expand Down Expand Up @@ -1042,7 +1042,7 @@ test('sub-application', async () => {
assert.ok('runs the sub-application /:id route');
assert.is(req.params.bar, 'hi', '~> parses the sub-application params');
assert.is(req.url, '/hi?a=0', '~> trims basepath from `req.url` value');
assert.equal(req.query, {a:'0'}, '~> parses the sub-application `res.query` value');
assert.equal({ ...req.query }, { a:'0' }, '~> parses the sub-application `res.query` value');
assert.is(req.originalUrl, '/sub/hi?a=0', '~> preserves original `req.url` value');
assert.is(req.foo, 'hello', '~> receives mutatations from main-app middleware');
assert.is(req.bar, 'world', '~> receives mutatations from own middleware');
Expand Down Expand Up @@ -1095,7 +1095,7 @@ test('sub-application w/ query params', async () => {
assert.ok('runs the sub-application / route');
assert.is(req.url, '/?foo=bar', '~> trims basepath from `req.url` value');
assert.is(req.originalUrl, '/sub?foo=bar', '~> preserves original `req.url` value');
assert.equal(req.query, { foo: 'bar' }, '~> preserves original `req.query` value');
assert.equal({ ...req.query }, { foo: 'bar' }, '~> preserves original `req.query` value');
res.end('hello from sub@index');
})
);
Expand All @@ -1107,7 +1107,7 @@ test('sub-application w/ query params', async () => {
assert.ok('run the main-application route');
assert.is(req.url, '/?foo=123', '~> always sets `req.originalUrl` key');
assert.is(req.originalUrl, '/?foo=123', '~> always sets `req.originalUrl` key');
assert.equal(req.query, { foo: '123' }, '~> sets the `req.query` value');
assert.equal({ ...req.query }, { foo: '123' }, '~> sets the `req.query` value');
res.end('hello from main');
})
);
Expand Down Expand Up @@ -1388,7 +1388,7 @@ test('decode url', async () => {
assert.ok('~> inside "GET /sub/:foo" handler')
assert.ok(req._decoded, '~> marked as decoded');
assert.is(req.path, '/føøß∂r', '~> decoded "path" value');
assert.is(req.url, '/føøß∂r?phone=+8675309', '~> decoded "url" value fully');
assert.is(req.url, '/føøß∂r?phone=%2b8675309', '~> decoded "url" value partially');
assert.is(req.params.foo, 'føøß∂r', '~> decoded "params.foo" segment');
assert.is(req.query.phone, '+8675309', '~~> does NOT decode "req.query" keys twice');
res.end('done');
Expand Down
285 changes: 211 additions & 74 deletions packages/url/bench/index.js
Original file line number Diff line number Diff line change
@@ -1,80 +1,217 @@
/* eslint-disable no-console */
global.native = require('url');
const native = require('url');
const qs = require('querystring');
const assert = require('uvu/assert');
const { Suite } = require('benchmark');
global.querystring = require('querystring');
global.parseurl = require('parseurl');
global.parse = require('../build');

global.nativeDecode = function (url) {
let obj = global.native.parse(url, true);
obj.href = obj.path = decodeURIComponent(url);
obj.pathname = decodeURIComponent(obj.pathname);
obj.search = decodeURIComponent(obj.search);
return obj;
}

global.parseurlDecode = function (req) {
let obj = global.parseurl(req);
obj.query = global.querystring.parse(obj.query);
obj.path = obj.href = decodeURIComponent(obj.href);
obj.pathname = decodeURIComponent(obj.pathname);
obj.search = decodeURIComponent(obj.search);
return obj;
}
const polka = require('../build').parse;
const parseurl = require('parseurl');

// Print validation error details
const isVerbose = process.argv.includes('--verbose');

/**
* @typedef Request
* @property {string} url
*/

/**
* @typedef Contender
* @type {(req: Request, toDecode: boolean) => any}
*/

/**
* All benchmark candidates!
* @NOTE Some are modified for decoding validation
* @type {Record<string, Contender>}
*/
const contenders = {
'url.parse': (r, d) => {
let out = native.parse(r.url, true); // query as object
if (d) out.pathname = decodeURIComponent(out.pathname);
// returns `null` if no value
out.search = out.search || '';
return out;
},

'new URL()': (r, d) => {
let url = r.url;
let { pathname, search, searchParams } = new URL(url, 'http://x.com');
if (d) pathname = decodeURIComponent(pathname);
return { url, pathname, search, query: searchParams };
},

'parseurl': (r, d) => {
/** @type {Record<string, string>} */ // @ts-ignore
let out = parseurl(r);

// returns `null` if no value
out.search = out.search || '';

// @ts-ignore - always returns `query` as string|null
if (out.query) out.query = qs.parse(out.query);

// never decodes, do bare minimum for compat
if (d) out.pathname = decodeURIComponent(out.pathname);

return out;
},

'@polka/url': (r, d) => {
// @ts-ignore - request
return polka(r, d);
},
};

/**
* @param {object} config
* @param {string} config.url
* @param {boolean} config.decode
* @param {Record<string, unknown>} config.expect
* @param {boolean} [config.repeat]
*/
function runner(config) {
let { url, expect, repeat, decode=false } = config;
let title = repeat ? 'repeat' : decode ? 'decode' : 'normal';

console.log('\nValidation: (%s) "%s"', title, url);
Object.keys(contenders).forEach(name => {
let key, fn=contenders[name];

function bench(name, url, setup='') {
global.url = url;
const suite = new Suite();
console.log(`\n# ${name} "${url}"`);
suite.with = (x, y) => suite.add(x, y, { setup, minSamples:100 });
suite.on('cycle', e => console.log(' ' + e.target));
return suite;
try {
let output = fn({ url }, decode);

for (key in expect) {
let tmp = output[key];
if (tmp instanceof URLSearchParams) {
tmp = Object.fromEntries(tmp);
} else if (tmp && typeof tmp === 'object') {
tmp = { ...tmp }; // null prototypes
}
assert.equal(tmp, expect[key]);
}
console.log(' ✔', name);
} catch (err) {
console.log(' ✘', name, `(FAILED @ "${key}")`);
if (isVerbose) console.log(err.details);
}
});

console.log('\nBenchmark: (%s) "%s"', title, url);
let bench = new Suite().on('cycle', e => {
console.log(' ' + e.target);
});

Object.keys(contenders).forEach(name => {
let fn = contenders[name];
let req = repeat && { url };

bench.add(name + ' '.repeat(16 - name.length), () => {
fn(req || { url }, decode);
}, {
minSamples: 100
});
});

bench.run();
}

bench('Parsing:', '/foo/bar?user=tj&pet=fluffy')
.with('native ', 'native.parse(url)')
.with('parseurl ', 'parseurl({ url })')
.with('@polka/url ', 'parse({ url })')
.run();

bench('REPEAT:', '/foo/bar?user=tj&pet=fluffy', 'req = { url }')
.with('native ', 'native.parse(req.url)')
.with('parseurl ', 'parseurl(req)')
.with('@polka/url ', 'parse(req)')
.run();

bench('Parsing:', '/foo/bar')
.with('native ', 'native.parse(url)')
.with('parseurl ', 'parseurl({ url })')
.with('@polka/url ', 'parse({ url })')
.run();

bench('Parsing:', '/')
.with('native ', 'native.parse(url)')
.with('parseurl ', 'parseurl({ url })')
.with('@polka/url ', 'parse({ url })')
.run();

bench('DECODE:', '/f%C3%B8%C3%B8%C3%9F%E2%88%82r')
.with('native/bad#1', 'native.parse(url, true)') // wrong
.with('native/bad#2', 'native.parse(decodeURIComponent(url), true)') // wrong
.with('native ', 'nativeDecode(url)')
.with('parseurl ', 'parseurlDecode({ url })')
.with('@polka/url ', 'parse({ url }, true)')
.run();

bench('DECODE:', '/f%C3%B8%C3%B8%C3%9F%E2%88%82r?phone=%2b393383123549')
.with('native/bad#1', 'native.parse(url, true)') // wrong
.with('native/bad#2', 'native.parse(decodeURIComponent(url), true)') // wrong
.with('native ', 'nativeDecode(url)')
.with('parseurl ', 'parseurlDecode({ url })')
.with('@polka/url ', 'parse({ url }, true)')
.run();

bench('DECODE:', '/foo/bar')
.with('native/bad#1', 'native.parse(url, true)') // wrong
.with('native/bad#2', 'native.parse(decodeURIComponent(url), true)') // wrong
.with('native ', 'nativeDecode(url)')
.with('parseurl ', 'parseurlDecode({ url })')
.with('@polka/url ', 'parse({ url }, true)')
.run();
// ---

runner({
url: '/foo/bar?user=tj&pet=fluffy',
decode: false,
expect: {
pathname: '/foo/bar',
search: '?user=tj&pet=fluffy',
query: {
user: 'tj',
pet: 'fluffy',
}
}
});

runner({
repeat: true,
url: '/foo/bar?user=tj&pet=fluffy',
decode: false,
expect: {
pathname: '/foo/bar',
search: '?user=tj&pet=fluffy',
query: {
user: 'tj',
pet: 'fluffy',
}
}
});

runner({
url: '/foo/bar',
decode: false,
expect: {
pathname: '/foo/bar',
search: '',
}
});

runner({
url: '/',
decode: false,
expect: {
pathname: '/',
search: '',
}
});

// DECODES

runner({
url: '/f%C3%B8%C3%B8%C3%9F%E2%88%82r',
decode: true,
expect: {
pathname: '/føøß∂r',
search: '',
}
});

runner({
url: '/f%C3%B8%C3%B8%C3%9F%E2%88%82r?phone=%2b393383123549',
decode: true,
expect: {
pathname: '/føøß∂r',
search: '?phone=%2b393383123549',
query: { phone: '+393383123549' },
}
});

runner({
repeat: true,
url: '/f%C3%B8%C3%B8%C3%9F%E2%88%82r?phone=%2b393383123549',
decode: true,
expect: {
pathname: '/føøß∂r',
search: '?phone=%2b393383123549',
query: { phone: '+393383123549' },
}
});

runner({
url: '/foo/bar?hello=123',
decode: true,
expect: {
pathname: '/foo/bar',
search: '?hello=123',
query: {
hello: '123',
}
}
});

runner({
url: '/foo/bar',
decode: true,
expect: {
pathname: '/foo/bar',
search: '',
}
});
2 changes: 2 additions & 0 deletions packages/url/bench/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{
"private": true,
"devDependencies": {
"@types/benchmark": "2.1.0",
"@types/parseurl": "1.3.1",
"benchmark": "2.1.4",
"parseurl": "1.3.2"
}
Expand Down
Loading

0 comments on commit 66f9cc5

Please sign in to comment.