diff --git a/doc/api/http.markdown b/doc/api/http.markdown index e93aa2d2ddf..d992bdffa3e 100644 --- a/doc/api/http.markdown +++ b/doc/api/http.markdown @@ -413,7 +413,9 @@ Options: - `socketPath`: Unix Domain Socket (use one of host:port or socketPath) - `method`: A string specifying the HTTP request method. Defaults to `'GET'`. - `path`: Request path. Defaults to `'/'`. Should include query string if any. - E.G. `'/index.html?page=12'` + E.G. `'/index.html?page=12'`. An exception is thrown when the request path + contains illegal characters. Currently, only spaces are rejected but that + may change in the future. - `headers`: An object containing request headers. - `auth`: Basic authentication i.e. `'user:password'` to compute an Authorization header. diff --git a/lib/http.js b/lib/http.js index c45af98b9de..a1abd1e2e0e 100644 --- a/lib/http.js +++ b/lib/http.js @@ -52,11 +52,14 @@ var ClientRequest = exports.ClientRequest = client.ClientRequest; exports.request = function(options, cb) { if (typeof options === 'string') { options = url.parse(options); - } else if (options && options.path) { - options = util._extend({}, options); - options.path = encodeURI(options.path); - // encodeURI() doesn't escape quotes while url.parse() does. Fix up. - options.path = options.path.replace(/'/g, '%27'); + } else if (options && options.path && / /.test(options.path)) { + // The actual regex is more like /[^A-Za-z0-9\-._~!$&'()*+,;=/:@]/ + // with an additional rule for ignoring percentage-escaped characters + // but that's a) hard to capture in a regular expression that performs + // well, and b) possibly too restrictive for real-world usage. That's + // why it only scans for spaces because those are guaranteed to create + // an invalid request. + throw new TypeError('Request path contains unescaped characters.'); } if (options.protocol && options.protocol !== 'http:') { diff --git a/test/simple/test-http-client-escape-path.js b/test/simple/test-http-client-unescaped-path.js similarity index 50% rename from test/simple/test-http-client-escape-path.js rename to test/simple/test-http-client-unescaped-path.js index 0178c823ba2..376bca19c46 100644 --- a/test/simple/test-http-client-escape-path.js +++ b/test/simple/test-http-client-unescaped-path.js @@ -22,54 +22,8 @@ var common = require('../common'); var assert = require('assert'); var http = require('http'); -var util = require('util'); -first(); - -function first() { - test('/~username/', '/~username/', second); -} -function second() { - test('/\'foo bar\'', '/%27foo%20bar%27', third); -} -function third() { - var expected = '/%3C%3E%22%60%20%0D%0A%09%7B%7D%7C%5C%5E~%60%27'; - test('/<>"` \r\n\t{}|\\^~`\'', expected); -} - -function test(path, expected, next) { - function helper(arg, next) { - var server = http.createServer(function(req, res) { - assert.equal(req.url, expected); - res.end('OK'); - server.close(next); - }); - server.on('clientError', function(err) { - throw err; - }); - server.listen(common.PORT, '127.0.0.1', function() { - http.get(arg); - }); - } - - // Go the extra mile to ensure that the behavior of - // http.get("http://example.com/...") matches http.get({ path: ... }). - test1(); - - function test1() { - console.log('as url: ' + util.inspect(path)); - helper('http://127.0.0.1:' + common.PORT + path, test2); - } - function test2() { - var options = { - host: '127.0.0.1', - port: common.PORT, - path: path - }; - console.log('as options: ' + util.inspect(options)); - helper(options, done); - } - function done() { - if (next) next(); - } -} +assert.throws(function() { + // Path with spaces in it should throw. + http.get({ path: 'bad path' }, assert.fail); +}, /contains unescaped characters/);