From 913a6d0b9166da81e758b94fa3d60272ed9b6a80 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Tue, 4 Apr 2017 21:03:14 -0700 Subject: [PATCH 1/2] url: improve WHATWG URL inspection --- lib/internal/url.js | 65 ++++++++----- test/parallel/test-whatwg-url-inspect.js | 112 ++++++++++------------- 2 files changed, 88 insertions(+), 89 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index 9a70838c30d4a1..a1c7730bbd6724 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -255,33 +255,41 @@ class URL { } [util.inspect.custom](depth, opts) { + if (this == null || + Object.getPrototypeOf(this[context]) !== URLContext.prototype) { + throw new TypeError('Value of `this` is not a URL'); + } + const ctx = this[context]; - var ret = 'URL {\n'; - ret += ` href: ${this.href}\n`; - if (ctx.scheme !== undefined) - ret += ` protocol: ${this.protocol}\n`; - if (ctx.username !== undefined) - ret += ` username: ${this.username}\n`; - if (ctx.password !== undefined) { - const pwd = opts.showHidden ? ctx.password : '--------'; - ret += ` password: ${pwd}\n`; - } - if (ctx.host !== undefined) - ret += ` hostname: ${this.hostname}\n`; - if (ctx.port !== undefined) - ret += ` port: ${this.port}\n`; - if (ctx.path !== undefined) - ret += ` pathname: ${this.pathname}\n`; - if (ctx.query !== undefined) - ret += ` search: ${this.search}\n`; - if (ctx.fragment !== undefined) - ret += ` hash: ${this.hash}\n`; + + if (typeof depth === 'number' && depth < 0) + return opts.stylize('[Object]', 'special'); + + const obj = Object.assign(new URLTemp(), { + href: this.href, + origin: this.origin, + protocol: this.protocol, + username: this.username, + password: (opts.showHidden || ctx.password == null) ? + this.password : '--------', + host: this.host, + hostname: this.hostname, + port: this.port, + pathname: this.pathname, + search: this.search, + searchParams: this.searchParams, + hash: this.hash + }); if (opts.showHidden) { - ret += ` cannot-be-base: ${this[cannotBeBase]}\n`; - ret += ` special: ${this[special]}\n`; + obj.cannotBeBase = this[cannotBeBase]; + obj.special = this[special]; + obj[context] = this[context]; } - ret += '}'; - return ret; + + return util.inspect(obj, opts).replace(/^URLTemp/, + () => this.constructor.name); + + function URLTemp() {} } } @@ -891,6 +899,9 @@ class URLSearchParams { throw new TypeError('Value of `this` is not a URLSearchParams'); } + if (typeof recurseTimes === 'number' && recurseTimes < 0) + return ctx.stylize('[Object]', 'special'); + const separator = ', '; const innerOpts = Object.assign({}, ctx); if (recurseTimes !== null) { @@ -1244,6 +1255,12 @@ defineIDLClass(URLSearchParamsIteratorPrototype, 'URLSearchParamsIterator', { }; }, [util.inspect.custom](recurseTimes, ctx) { + if (this == null || this[context] == null || this[context].target == null) + throw new TypeError('Value of `this` is not a URLSearchParamsIterator'); + + if (typeof recurseTimes === 'number' && recurseTimes < 0) + return ctx.stylize('[Object]', 'special'); + const innerOpts = Object.assign({}, ctx); if (recurseTimes !== null) { innerOpts.depth = recurseTimes - 1; diff --git a/test/parallel/test-whatwg-url-inspect.js b/test/parallel/test-whatwg-url-inspect.js index 4afbbc13102905..e278b5b441bf1e 100644 --- a/test/parallel/test-whatwg-url-inspect.js +++ b/test/parallel/test-whatwg-url-inspect.js @@ -13,71 +13,53 @@ if (!common.hasIntl) { } // Tests below are not from WPT. -const tests = require(path.join(common.fixturesDir, 'url-tests')); -const additional_tests = require( - path.join(common.fixturesDir, 'url-tests-additional')); +const url = new URL('https://username:password@host.name:8080/path/name/?que=ry#hash'); -const allTests = additional_tests.slice(); -for (const test of tests) { - if (test.failure || typeof test === 'string') continue; - allTests.push(test); -} - -for (const test of allTests) { - const url = test.url ? new URL(test.url) : new URL(test.input, test.base); - - for (const showHidden of [true, false]) { - const res = util.inspect(url, { - showHidden - }); - - const lines = res.split('\n'); - - const firstLine = lines[0]; - assert.strictEqual(firstLine, 'URL {'); +assert.strictEqual( + util.inspect(url), + `URL { + href: 'https://username:password@host.name:8080/path/name/?que=ry#hash', + origin: 'https://host.name:8080', + protocol: 'https:', + username: 'username', + password: '--------', + host: 'host.name:8080', + hostname: 'host.name', + port: '8080', + pathname: '/path/name/', + search: '?que=ry', + searchParams: URLSearchParams { 'que' => 'ry' }, + hash: '#hash' }`); - const lastLine = lines[lines.length - 1]; - assert.strictEqual(lastLine, '}'); +assert.strictEqual( + util.inspect(url, { showHidden: true }), + `URL { + href: 'https://username:password@host.name:8080/path/name/?que=ry#hash', + origin: 'https://host.name:8080', + protocol: 'https:', + username: 'username', + password: 'password', + host: 'host.name:8080', + hostname: 'host.name', + port: '8080', + pathname: '/path/name/', + search: '?que=ry', + searchParams: URLSearchParams { 'que' => 'ry' }, + hash: '#hash', + cannotBeBase: false, + special: true, + [Symbol(context)]:\x20 + URLContext { + flags: 2032, + scheme: 'https:', + username: 'username', + password: 'password', + host: 'host.name', + port: 8080, + path: [ 'path', 'name', '', [length]: 3 ], + query: 'que=ry', + fragment: 'hash' } }`); - const innerLines = lines.slice(1, lines.length - 1); - const keys = new Set(); - for (const line of innerLines) { - const i = line.indexOf(': '); - const k = line.slice(0, i).trim(); - const v = line.slice(i + 2); - assert.strictEqual(keys.has(k), false, 'duplicate key found: ' + k); - keys.add(k); - - const hidden = new Set([ - 'password', - 'cannot-be-base', - 'special' - ]); - if (showHidden) { - if (!hidden.has(k)) { - assert.strictEqual(v, url[k], k); - continue; - } - - if (k === 'password') { - assert.strictEqual(v, url[k], k); - } - if (k === 'cannot-be-base') { - assert.ok(v.match(/^true$|^false$/), k + ' is Boolean'); - } - if (k === 'special') { - assert.ok(v.match(/^true$|^false$/), k + ' is Boolean'); - } - continue; - } - - // showHidden is false - if (k === 'password') { - assert.strictEqual(v, '--------', k); - continue; - } - assert.strictEqual(hidden.has(k), false, 'no hidden keys: ' + k); - assert.strictEqual(v, url[k], k); - } - } -} +assert.strictEqual( + util.inspect({ a: url }, { depth: 0 }), + '{ a: [Object] }'); From 0f48f3e17fa8d2d54fbfa9560bb8ce7673ee2f99 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Thu, 6 Apr 2017 22:39:04 -0700 Subject: [PATCH 2/2] Avoid using hacky temporary constructor --- lib/internal/url.js | 49 +++++++++++++++--------- test/parallel/test-whatwg-url-inspect.js | 4 +- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index a1c7730bbd6724..9f8cb323e9b467 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -235,6 +235,17 @@ function onParseHashComplete(flags, protocol, username, password, } } +function getEligibleConstructor(obj) { + while (obj !== null) { + if (Object.prototype.hasOwnProperty.call(obj, 'constructor') && + typeof obj.constructor === 'function') { + return obj.constructor; + } + obj = Object.getPrototypeOf(obj); + } + return null; +} + class URL { constructor(input, base) { // toUSVString is not needed. @@ -265,31 +276,33 @@ class URL { if (typeof depth === 'number' && depth < 0) return opts.stylize('[Object]', 'special'); - const obj = Object.assign(new URLTemp(), { - href: this.href, - origin: this.origin, - protocol: this.protocol, - username: this.username, - password: (opts.showHidden || ctx.password == null) ? - this.password : '--------', - host: this.host, - hostname: this.hostname, - port: this.port, - pathname: this.pathname, - search: this.search, - searchParams: this.searchParams, - hash: this.hash + const ctor = getEligibleConstructor(this); + + const obj = Object.create({ + constructor: ctor === null ? URL : ctor }); + + obj.href = this.href; + obj.origin = this.origin; + obj.protocol = this.protocol; + obj.username = this.username; + obj.password = (opts.showHidden || ctx.password == null) ? + this.password : '--------'; + obj.host = this.host; + obj.hostname = this.hostname; + obj.port = this.port; + obj.pathname = this.pathname; + obj.search = this.search; + obj.searchParams = this.searchParams; + obj.hash = this.hash; + if (opts.showHidden) { obj.cannotBeBase = this[cannotBeBase]; obj.special = this[special]; obj[context] = this[context]; } - return util.inspect(obj, opts).replace(/^URLTemp/, - () => this.constructor.name); - - function URLTemp() {} + return util.inspect(obj, opts); } } diff --git a/test/parallel/test-whatwg-url-inspect.js b/test/parallel/test-whatwg-url-inspect.js index e278b5b441bf1e..a8a59b77873f12 100644 --- a/test/parallel/test-whatwg-url-inspect.js +++ b/test/parallel/test-whatwg-url-inspect.js @@ -3,7 +3,6 @@ const common = require('../common'); const util = require('util'); const URL = require('url').URL; -const path = require('path'); const assert = require('assert'); if (!common.hasIntl) { @@ -63,3 +62,6 @@ assert.strictEqual( assert.strictEqual( util.inspect({ a: url }, { depth: 0 }), '{ a: [Object] }'); + +class MyURL extends URL {} +assert(util.inspect(new MyURL(url.href)).startsWith('MyURL {'));