Skip to content

Commit

Permalink
fix: accept absolute urls as path (#468)
Browse files Browse the repository at this point in the history
  • Loading branch information
szmarczak authored and ronag committed Nov 12, 2020
1 parent 1a457c7 commit a991989
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 deletions.
10 changes: 8 additions & 2 deletions lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ const kRequestTimeout = Symbol('request timeout')
const kTimeout = Symbol('timeout')
const kHandler = Symbol('handler')

// Borrowed from https://gist.github.com/dperini/729294
// eslint-disable-next-line no-control-regex
const REGEXP_ABSOLUTE_URL = /^(?:(?:https?|ftp):\/\/)(?:\S+(?::\S*)?@)?(?:(?!10(?:\.\d{1,3}){3})(?!127(?:\.\d{1,3}){3})(?!169\.254(?:\.\d{1,3}){2})(?!192\.168(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\x00a1-\xffff0-9]+-?)*[a-z\x00a1-\xffff0-9]+)(?:\.(?:[a-z\x00a1-\xffff0-9]+-?)*[a-z\x00a1-\xffff0-9]+)*(?:\.(?:[a-z\x00a1-\xffff]{2,})))(?::\d{2,5})?(?:\/[^\s]*)?$/ius

class Request {
constructor ({
path,
Expand All @@ -23,8 +27,10 @@ class Request {
upgrade,
requestTimeout
}, handler) {
if (typeof path !== 'string' || path[0] !== '/') {
throw new InvalidArgumentError('path must be a valid path')
if (typeof path !== 'string') {
throw new InvalidArgumentError('path must be a string')
} else if (path[0] !== '/' && !REGEXP_ABSOLUTE_URL.test(path)) {
throw new InvalidArgumentError('path must be an absolute URL or start with a slash')
}

if (typeof method !== 'string') {
Expand Down
26 changes: 26 additions & 0 deletions test/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,32 @@ test('url-like url', (t) => {
})
})

test('an absolute url as path', (t) => {
t.plan(2)

const path = 'http://example.com'

const server = createServer((req, res) => {
t.strictEqual(req.url, path)
res.end()
})
t.tearDown(server.close.bind(server))

server.listen(0, () => {
const client = new Client({
hostname: 'localhost',
port: server.address().port,
protocol: 'http'
})
t.tearDown(client.close.bind(client))

client.request({ path, method: 'GET' }, (err, data) => {
t.error(err)
data.body.resume()
})
})
})

test('multiple destroy callback', (t) => {
t.plan(4)

Expand Down
4 changes: 2 additions & 2 deletions test/validations.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ server.listen(0, () => {

client.request({ path: null, method: 'GET' }, (err, res) => {
t.ok(err instanceof errors.InvalidArgumentError)
t.strictEqual(err.message, 'path must be a valid path')
t.strictEqual(err.message, 'path must be a string')
})

client.request({ path: 'aaa', method: 'GET' }, (err, res) => {
t.ok(err instanceof errors.InvalidArgumentError)
t.strictEqual(err.message, 'path must be a valid path')
t.strictEqual(err.message, 'path must be an absolute URL or start with a slash')
})
})

Expand Down

0 comments on commit a991989

Please sign in to comment.