Remove JSONP support. Closes #4270 (#4271)

hapijs · Apr 18, 2022 · c44e73e · c44e73e
1 parent 2f80e84
commit c44e73e
Show file tree

Hide file tree

Showing 9 changed files with 4 additions and 267 deletions.
diff --git a/API.md b/API.md
@@ -2973,21 +2973,6 @@ string payload or escaping it after stringification. Supports the following:
 - `escape` - calls [`Hoek.jsonEscape()`](https://hapi.dev/family/hoek/api/#escapejsonstring)
   after conversion to JSON string. Defaults to `false`.
 
-### <a name="route.options.jsonp" /> `route.options.jsonp`
-
-Default value: none.
-
-Enables JSONP support by setting the value to the query parameter name containing the function name
-used to wrap the response payload.
-
-For example, if the value is `'callback'`, a request comes in with `'callback=me'`, and the JSON
-response is `'{ "a":"b" }'`, the payload will be `'me({ "a":"b" });'`. Cannot be used with stream
-responses.
-
-The 'Content-Type' response header is set to `'text/javascript'` and the 'X-Content-Type-Options'
-response header is set to `'nosniff'`, and will override those headers even if explicitly set by
-[`response.type()`](#response.type()).
-
 ### <a name="route.options.log" /> `route.options.log`
 
 Default value: `{ collect: false }`.
@@ -3615,19 +3600,12 @@ the same. The following is the complete list of steps a request can go through:
     - [`request.route`](#request.route) is unassigned.
     - [`request.url`](#request.url) can be `null` if the incoming request path is invalid.
     - [`request.path`](#request.path) can be an invalid path.
-    - JSONP configuration is ignored for any response returned from the extension point since no
-      route is matched yet and the JSONP configuration is unavailable.
 
 - _**Route lookup**_
     - lookup based on `request.path` and `request.method`.
     - skips to _**onPreResponse**_ if no route is found or if the path violates the HTTP
       specification.
 
-- _**JSONP processing**_
-    - based on the route [`jsonp`](#route.options.jsonp) option.
-    - parses JSONP parameter from [`request.query`](#request.query).
-    - skips to _**Response validation**_ on error.
-
 - _**Cookies processing**_
     - based on the route [`state`](#route.options.state) option.
     - error handling based on [`failAction`](#route.options.state.failAction).
@@ -3662,10 +3640,6 @@ the same. The following is the complete list of steps a request can go through:
     - based on the route [`validate.params`](#route.options.validate.params) option.
     - error handling based on [`failAction`](#route.options.validate.failAction).
 
-- _**JSONP cleanup**_
-    - based on the route [`jsonp`](#route.options.jsonp) option.
-    - remove the JSONP parameter from [`request.query`](#request.query).
-
 - _**Query validation**_
     - based on the route [`validate.query`](#route.options.validate.query) option.
     - error handling based on [`failAction`](#route.options.validate.failAction).

diff --git a/lib/config.js b/lib/config.js
@@ -134,7 +134,6 @@ internals.routeBase = Validate.object({
         escape: Validate.boolean().default(false)
     })
         .default(),
-    jsonp: Validate.string(),
     log: Validate.object({
         collect: Validate.boolean().default(false)
     })

diff --git a/lib/headers.js b/lib/headers.js
@@ -40,17 +40,7 @@ exports.content = async function (response) {
 
         await response._marshal();
 
-        if (request.jsonp &&
-            response._payload.jsonp) {
-
-            response._header('content-type', 'text/javascript' + (response.settings.charset ? '; charset=' + response.settings.charset : ''));
-            response._header('x-content-type-options', 'nosniff');
-            response._payload.jsonp(request.jsonp);
-        }
-
-        if (response._payload.size &&
-            typeof response._payload.size === 'function') {
-
+        if (typeof response._payload.size === 'function') {
             response._header('content-length', response._payload.size(), { override: false });
         }
 

diff --git a/lib/request.js b/lib/request.js
@@ -15,7 +15,7 @@ const Transmit = require('./transmit');
 
 const internals = {
     events: Podium.validate(['finish', { name: 'peek', spread: true }, 'disconnect']),
-    reserved: ['server', 'url', 'query', 'path', 'method', 'mime', 'setUrl', 'setMethod', 'headers', 'id', 'app', 'plugins', 'route', 'auth', 'pre', 'preResponses', 'info', 'isInjected', 'orig', 'params', 'paramsArray', 'payload', 'state', 'jsonp', 'response', 'raw', 'domain', 'log', 'logs', 'generateResponse']
+    reserved: ['server', 'url', 'query', 'path', 'method', 'mime', 'setUrl', 'setMethod', 'headers', 'id', 'app', 'plugins', 'route', 'auth', 'pre', 'preResponses', 'info', 'isInjected', 'orig', 'params', 'paramsArray', 'payload', 'state', 'response', 'raw', 'domain', 'log', 'logs', 'generateResponse']
 };
 
 
@@ -41,7 +41,6 @@ exports = module.exports = internals.Request = class {
 
         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)
         this.headers = req.headers;
-        this.jsonp = null;
         this.logs = [];
         this.method = req.method.toLowerCase();
         this.mime = null;

diff --git a/lib/response.js b/lib/response.js
@@ -711,68 +711,33 @@ internals.Response.Payload = class extends Stream.Readable {
         super();
 
         this._data = payload;
-        this._prefix = null;
-        this._suffix = null;
-        this._sizeOffset = 0;
         this._encoding = options.encoding;
     }
 
     _read(size) {
 
-        if (this._prefix) {
-            this.push(this._prefix, this._encoding);
-        }
-
         if (this._data) {
             this.push(this._data, this._encoding);
         }
 
-        if (this._suffix) {
-            this.push(this._suffix, this._encoding);
-        }
-
         this.push(null);
     }
 
     size() {
 
         if (!this._data) {
-            return this._sizeOffset;
+            return 0;
         }
 
-        return (Buffer.isBuffer(this._data) ? this._data.length : Buffer.byteLength(this._data, this._encoding)) + this._sizeOffset;
-    }
-
-    jsonp(variable) {
-
-        this._sizeOffset = this._sizeOffset + variable.length + 7;
-        this._prefix = '/**/' + variable + '(';                 // '/**/' prefix prevents CVE-2014-4671 security exploit
-
-        if (this._data !== null &&
-            !Buffer.isBuffer(this._data)) {
-
-            this._data = this._data
-                .replace(/\u2028/g, '\\u2028')
-                .replace(/\u2029/g, '\\u2029');
-        }
-
-        this._suffix = ');';
+        return Buffer.isBuffer(this._data) ? this._data.length : Buffer.byteLength(this._data, this._encoding);
     }
 
     writeToStream(stream) {
 
-        if (this._prefix) {
-            stream.write(this._prefix, this._encoding);
-        }
-
         if (this._data) {
             stream.write(this._data, this._encoding);
         }
 
-        if (this._suffix) {
-            stream.write(this._suffix, this._encoding);
-        }
-
         stream.end();
     }
 };

diff --git a/lib/route.js b/lib/route.js
@@ -2,7 +2,6 @@
 
 const Assert = require('assert');
 
-const Boom = require('@hapi/boom');
 const Bounce = require('@hapi/bounce');
 const Catbox = require('@hapi/catbox');
 const Hoek = require('@hapi/hoek');
@@ -127,7 +126,6 @@ exports = module.exports = internals.Route = class {
 
         this._assert(!this.settings.validate.payload || this.settings.payload.parse, 'Route payload must be set to \'parse\' when payload validation enabled');
         this._assert(!this.settings.validate.state || this.settings.state.parse, 'Route state must be set to \'parse\' when state validation enabled');
-        this._assert(!this.settings.jsonp || typeof this.settings.jsonp === 'string', 'Bad route JSONP parameter name');
 
         // Authentication configuration
 
@@ -232,10 +230,6 @@ exports = module.exports = internals.Route = class {
 
         // 'onRequest'
 
-        if (this.settings.jsonp) {
-            this._cycle.push(internals.parseJSONP);
-        }
-
         if (this.settings.state.parse) {
             this._cycle.push(internals.state);
         }
@@ -278,10 +272,6 @@ exports = module.exports = internals.Route = class {
             this._cycle.push(Validation.params);
         }
 
-        if (this.settings.jsonp) {
-            this._cycle.push(internals.cleanupJSONP);
-        }
-
         if (this.settings.validate.query) {
             this._cycle.push(Validation.query);
         }
@@ -457,30 +447,6 @@ internals.drain = async function (request) {
 };
 
 
-internals.jsonpRegex = /^[\w\$\[\]\.]+$/;
-
-
-internals.parseJSONP = function (request) {
-
-    const jsonp = request.query[request.route.settings.jsonp];
-    if (jsonp) {
-        if (internals.jsonpRegex.test(jsonp) === false) {
-            throw Boom.badRequest('Invalid JSONP parameter value');
-        }
-
-        request.jsonp = jsonp;
-    }
-};
-
-
-internals.cleanupJSONP = function (request) {
-
-    if (request.jsonp) {
-        delete request.query[request.route.settings.jsonp];
-    }
-};
-
-
 internals.config = function (chain) {
 
     if (!chain.length) {

diff --git a/test/headers.js b/test/headers.js
@@ -6,7 +6,6 @@ const Code = require('@hapi/code');
 const Hapi = require('..');
 const Inert = require('@hapi/inert');
 const Lab = require('@hapi/lab');
-const Wreck = require('@hapi/wreck');
 
 
 const internals = {};
@@ -513,17 +512,6 @@ describe('Headers', () => {
             expect(res.headers['content-type']).to.equal('text/html');
         });
 
-        it('returns a normal response when JSONP requested but stream returned', async () => {
-
-            const server = Hapi.server();
-            const stream = Wreck.toReadableStream('test');
-            stream.size = 4;                                    // Non function for coverage
-            server.route({ method: 'GET', path: '/', options: { jsonp: 'callback', handler: () => stream } });
-
-            const res = await server.inject('/?callback=me');
-            expect(res.payload).to.equal('test');
-        });
-
         it('does not set content-type by default on 204 response', async () => {
 
             const server = Hapi.server();

diff --git a/test/response.js b/test/response.js
@@ -1553,16 +1553,4 @@ describe('Response', () => {
             await finish;
         });
     });
-
-    describe('Payload', () => {
-
-        it('streams empty string', async () => {
-
-            const server = Hapi.server({ compression: { minBytes: 1 } });
-            server.route({ method: 'GET', path: '/', options: { jsonp: 'callback', handler: () => '' } });
-
-            const res = await server.inject({ url: '/?callback=me', headers: { 'Accept-Encoding': 'gzip' } });
-            expect(res.statusCode).to.equal(200);
-        });
-    });
 });