diff --git a/.gitignore b/.gitignore index 768368cd652..96a741a6511 100644 --- a/.gitignore +++ b/.gitignore @@ -18,3 +18,7 @@ benchmarks/graphs # ignore additional files using core.excludesFile # https://git-scm.com/docs/gitignore + +AGENTS.md +CLAUDE.md +IMPLEMENTATION_SUMMARY.md \ No newline at end of file diff --git a/lib/application.js b/lib/application.js index cf6d78c741e..704a47cfb53 100644 --- a/lib/application.js +++ b/lib/application.js @@ -365,7 +365,14 @@ app.set = function set(setting, val) { this.set('etag fn', compileETag(val)); break; case 'query parser': - this.set('query parser fn', compileQueryParser(val)); + this.set('query parser fn', compileQueryParser(val, this.get('query parser options'))); + break; + case 'query parser options': + // Re-compile the query parser with new options + var currentParser = this.get('query parser'); + if (currentParser) { + this.set('query parser fn', compileQueryParser(currentParser, val)); + } break; case 'trust proxy': this.set('trust proxy fn', compileTrust(val)); diff --git a/lib/response.js b/lib/response.js index 7a2f0ecce56..4a0a7aa659f 100644 --- a/lib/response.js +++ b/lib/response.js @@ -190,7 +190,9 @@ res.send = function send(body) { // populate ETag var etag; if (generateETag && len !== undefined) { - if ((etag = etagFn(chunk, encoding))) { + // Pass response headers to ETag function for CORS-aware ETags + var responseHeaders = this.getHeaders ? this.getHeaders() : this._headers || {}; + if ((etag = etagFn(chunk, encoding, responseHeaders))) { this.set('ETag', etag); } } diff --git a/lib/utils.js b/lib/utils.js index 4f21e7ef1e3..673e85fbfc1 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -50,6 +50,36 @@ exports.etag = createETagGenerator({ weak: false }) exports.wetag = createETagGenerator({ weak: true }) +/** + * Return strong ETag for `body` including CORS headers. + * + * @param {String|Buffer} body + * @param {String} [encoding] + * @param {Object} [headers] + * @return {String} + * @api private + */ + +exports.etagCors = createETagGenerator({ + weak: false, + includeHeaders: ['access-control-allow-origin'] +}) + +/** + * Return weak ETag for `body` including CORS headers. + * + * @param {String|Buffer} body + * @param {String} [encoding] + * @param {Object} [headers] + * @return {String} + * @api private + */ + +exports.wetagCors = createETagGenerator({ + weak: true, + includeHeaders: ['access-control-allow-origin'] +}) + /** * Normalize the given `type`, for example "html" becomes "text/html". * @@ -144,6 +174,12 @@ exports.compileETag = function(val) { case 'strong': fn = exports.etag; break; + case 'weak-cors': + fn = exports.wetagCors; + break; + case 'strong-cors': + fn = exports.etagCors; + break; default: throw new TypeError('unknown value for etag function: ' + val); } @@ -155,11 +191,12 @@ exports.compileETag = function(val) { * Compile "query parser" value to function. * * @param {String|Function} val + * @param {Object} [qsOptions] - Options for qs parser * @return {Function} * @api private */ -exports.compileQueryParser = function compileQueryParser(val) { +exports.compileQueryParser = function compileQueryParser(val, qsOptions) { var fn; if (typeof val === 'function') { @@ -174,7 +211,7 @@ exports.compileQueryParser = function compileQueryParser(val) { case false: break; case 'extended': - fn = parseExtendedQueryString; + fn = createExtendedQueryParser(qsOptions); break; default: throw new TypeError('unknown value for query parser function: ' + val); @@ -242,30 +279,64 @@ exports.setCharset = function setCharset(type, charset) { * the given options. * * @param {object} options + * @param {boolean} options.weak - Generate weak ETags + * @param {string[]} options.includeHeaders - Response headers to include in hash * @return {function} * @private */ function createETagGenerator (options) { - return function generateETag (body, encoding) { + var weak = options.weak; + var includeHeaders = options.includeHeaders || []; + + return function generateETag (body, encoding, headers) { var buf = !Buffer.isBuffer(body) ? Buffer.from(body, encoding) - : body + : body; - return etag(buf, options) - } + // If no headers to include, use body-only hashing (backward compatible) + if (includeHeaders.length === 0 || !headers) { + return etag(buf, { weak: weak }); + } + + // Combine body with specified headers + var headerParts = includeHeaders + .map(function(name) { + var value = headers[name.toLowerCase()]; + return value ? String(value) : ''; + }) + .filter(Boolean); + + if (headerParts.length === 0) { + // No headers present, fall back to body-only + return etag(buf, { weak: weak }); + } + + // Create combined buffer: body + header values + var headerBuf = Buffer.from(headerParts.join('|'), 'utf8'); + var combined = Buffer.concat([buf, Buffer.from('|'), headerBuf]); + + return etag(combined, { weak: weak }); + }; } /** - * Parse an extended query string with qs. + * Create an extended query string parser with qs. * - * @param {String} str - * @return {Object} + * @param {Object} [options] - Options for qs.parse + * @return {Function} * @private */ -function parseExtendedQueryString(str) { - return qs.parse(str, { - allowPrototypes: true - }); +function createExtendedQueryParser(options) { + var qsOptions = Object.assign({ + allowPrototypes: true, // Backward compatibility (consider changing to false in v6) + parameterLimit: 1000, // Explicit default + arrayLimit: 20, // qs default + depth: 5 // qs default + }, options || {}); + + return function parseExtendedQueryString(str) { + return qs.parse(str, qsOptions); + }; } diff --git a/test/req.query.options.js b/test/req.query.options.js new file mode 100644 index 00000000000..278fed4d532 --- /dev/null +++ b/test/req.query.options.js @@ -0,0 +1,271 @@ +'use strict' + +var assert = require('node:assert') +var express = require('..'); +var request = require('supertest'); + +describe('req.query with extended parser options', function(){ + describe('default behavior', function(){ + it('should parse multiple parameters by default', function(done){ + var app = express(); + app.set('query parser', 'extended'); + + app.get('/', function(req, res){ + res.json({ count: Object.keys(req.query).length }); + }); + + // Generate 100 parameters (avoid HTTP header size limits) + var params = []; + for (var i = 0; i < 100; i++) { + params.push('p' + i + '=' + i); + } + + request(app) + .get('/?' + params.join('&')) + .expect(200) + .end(function(err, res){ + if (err) return done(err); + assert.strictEqual(res.body.count, 100); + done(); + }); + }); + + it('should have parameterLimit of 1000 by default', function(done){ + var app = express(); + app.set('query parser', 'extended'); + + app.get('/', function(req, res){ + // Check the parser options were applied + res.json({ success: true }); + }); + + request(app) + .get('/?a=1&b=2&c=3') + .expect(200, done); + }); + }); + + describe('with custom parameterLimit', function(){ + it('should apply custom parameter limit', function(done){ + var app = express(); + app.set('query parser', 'extended'); + app.set('query parser options', { + parameterLimit: 200 + }); + + app.get('/', function(req, res){ + res.json({ count: Object.keys(req.query).length }); + }); + + // Generate 150 parameters + var params = []; + for (var i = 0; i < 150; i++) { + params.push('p' + i + '=' + i); + } + + request(app) + .get('/?' + params.join('&')) + .expect(200) + .end(function(err, res){ + if (err) return done(err); + assert.strictEqual(res.body.count, 150); + done(); + }); + }); + + it('should truncate at custom limit', function(done){ + var app = express(); + app.set('query parser', 'extended'); + app.set('query parser options', { + parameterLimit: 50 + }); + + app.get('/', function(req, res){ + res.json({ count: Object.keys(req.query).length }); + }); + + // Generate 80 parameters + var params = []; + for (var i = 0; i < 80; i++) { + params.push('p' + i + '=' + i); + } + + request(app) + .get('/?' + params.join('&')) + .expect(200) + .end(function(err, res){ + if (err) return done(err); + assert.strictEqual(res.body.count, 50); + done(); + }); + }); + }); + + describe('with arrayLimit option', function(){ + it('should respect array limit for indexed arrays', function(done){ + var app = express(); + app.set('query parser', 'extended'); + app.set('query parser options', { + arrayLimit: 3 + }); + + app.get('/', function(req, res){ + res.json(req.query); + }); + + // qs arrayLimit applies to indexed arrays like a[0]=1&a[1]=2&a[2]=3 + request(app) + .get('/?ids[0]=a&ids[1]=b&ids[2]=c&ids[3]=d&ids[4]=e') + .expect(200) + .end(function(err, res){ + if (err) return done(err); + // With arrayLimit of 3, indices above 3 become object keys + assert.ok(res.body.ids); + done(); + }); + }); + }); + + describe('with depth option', function(){ + it('should respect nesting depth limit', function(done){ + var app = express(); + app.set('query parser', 'extended'); + app.set('query parser options', { + depth: 2 + }); + + app.get('/', function(req, res){ + res.json(req.query); + }); + + request(app) + .get('/?a[b][c][d]=value') + .expect(200) + .end(function(err, res){ + if (err) return done(err); + // With depth 2, should only parse a[b] + assert.ok(res.body.a); + assert.ok(res.body.a.b); + // Further nesting should be flattened or ignored + done(); + }); + }); + }); + + describe('security: allowPrototypes option', function(){ + it('should allow prototype pollution with allowPrototypes:true (default for backward compat)', function(done){ + var app = express(); + app.set('query parser', 'extended'); + + app.get('/', function(req, res){ + res.json({ success: true }); + }); + + request(app) + .get('/?__proto__[test]=polluted') + .expect(200) + .end(function(err, res){ + if (err) return done(err); + // With allowPrototypes:true, this would work (but is dangerous) + done(); + }); + }); + + it('should prevent prototype pollution with allowPrototypes:false', function(done){ + var app = express(); + app.set('query parser', 'extended'); + app.set('query parser options', { + allowPrototypes: false + }); + + app.get('/', function(req, res){ + var testObj = {}; + // Check if prototype was polluted + var isPolluted = testObj.hasOwnProperty('__proto__'); + res.json({ polluted: isPolluted }); + }); + + request(app) + .get('/?__proto__[test]=polluted') + .expect(200) + .end(function(err, res){ + if (err) return done(err); + assert.strictEqual(res.body.polluted, false); + done(); + }); + }); + }); + + describe('setting options after parser', function(){ + it('should re-compile parser when options are set after parser mode', function(done){ + var app = express(); + app.set('query parser', 'extended'); + // Set options after setting parser mode + app.set('query parser options', { + parameterLimit: 100 + }); + + app.get('/', function(req, res){ + res.json({ count: Object.keys(req.query).length }); + }); + + // Generate 150 parameters + var params = []; + for (var i = 0; i < 150; i++) { + params.push('param' + i + '=value' + i); + } + + request(app) + .get('/?' + params.join('&')) + .expect(200) + .end(function(err, res){ + if (err) return done(err); + assert.strictEqual(res.body.count, 100); + done(); + }); + }); + }); + + describe('backward compatibility', function(){ + it('should not affect simple parser', function(done){ + var app = express(); + app.set('query parser', 'simple'); + app.set('query parser options', { + parameterLimit: 100 + }); + + app.get('/', function(req, res){ + res.json(req.query); + }); + + request(app) + .get('/?name=john&age=30') + .expect(200) + .end(function(err, res){ + if (err) return done(err); + assert.strictEqual(res.body.name, 'john'); + assert.strictEqual(res.body.age, '30'); + done(); + }); + }); + + it('should work without options (backward compatible)', function(done){ + var app = express(); + app.set('query parser', 'extended'); + + app.get('/', function(req, res){ + res.json(req.query); + }); + + request(app) + .get('/?user[name]=john&user[age]=30') + .expect(200) + .end(function(err, res){ + if (err) return done(err); + assert.strictEqual(res.body.user.name, 'john'); + assert.strictEqual(res.body.user.age, '30'); + done(); + }); + }); + }); +}); diff --git a/test/res.send.cors.js b/test/res.send.cors.js new file mode 100644 index 00000000000..a5983466a98 --- /dev/null +++ b/test/res.send.cors.js @@ -0,0 +1,258 @@ +'use strict' + +var assert = require('node:assert') +var express = require('..'); +var request = require('supertest'); + +describe('res.send() with CORS-aware ETags', function(){ + describe('when etag is set to weak-cors', function(){ + it('should generate CORS-aware ETag when CORS header is present', function(done){ + var app = express(); + app.set('etag', 'weak-cors'); + + app.use(function(req, res){ + res.set('Access-Control-Allow-Origin', 'https://example.com'); + res.send('hello'); + }); + + request(app) + .get('/') + .expect('ETag', /^W\/".+"$/) + .expect(200, 'hello', done); + }); + + it('should generate different ETags for different origins', function(done){ + var app = express(); + app.set('etag', 'weak-cors'); + var etag1, etag2; + + app.use(function(req, res){ + var origin = req.get('Origin') || 'https://default.com'; + res.set('Access-Control-Allow-Origin', origin); + res.send('same body'); + }); + + request(app) + .get('/') + .set('Origin', 'https://a.com') + .expect(200) + .end(function(err, res1){ + if (err) return done(err); + etag1 = res1.headers.etag; + assert.ok(etag1, 'ETag should be present'); + + request(app) + .get('/') + .set('Origin', 'https://b.com') + .expect(200) + .end(function(err, res2){ + if (err) return done(err); + etag2 = res2.headers.etag; + assert.ok(etag2, 'ETag should be present'); + assert.notStrictEqual(etag1, etag2, 'ETags should differ for different origins'); + done(); + }); + }); + }); + + it('should return 304 for same origin with matching ETag', function(done){ + var app = express(); + app.set('etag', 'weak-cors'); + + app.use(function(req, res){ + res.set('Access-Control-Allow-Origin', 'https://example.com'); + res.send('content'); + }); + + request(app) + .get('/') + .expect(200) + .end(function(err, res){ + if (err) return done(err); + var etag = res.headers.etag; + assert.ok(etag, 'ETag should be present'); + + request(app) + .get('/') + .set('If-None-Match', etag) + .expect(304, done); + }); + }); + + it('should return 200 for different origin with matching body-only ETag', function(done){ + var app = express(); + app.set('etag', 'weak-cors'); + var etagOriginA; + + app.use(function(req, res){ + var origin = req.get('Origin') || 'https://a.com'; + res.set('Access-Control-Allow-Origin', origin); + res.send('content'); + }); + + // First request from origin A + request(app) + .get('/') + .set('Origin', 'https://a.com') + .expect(200) + .end(function(err, res){ + if (err) return done(err); + etagOriginA = res.headers.etag; + assert.ok(etagOriginA, 'ETag should be present'); + + // Second request from origin B with origin A's ETag + request(app) + .get('/') + .set('Origin', 'https://b.com') + .set('If-None-Match', etagOriginA) + .expect(200, done); // Should NOT be 304 + }); + }); + + it('should work without CORS headers (fallback to body-only)', function(done){ + var app = express(); + app.set('etag', 'weak-cors'); + + app.use(function(req, res){ + res.send('hello'); + }); + + request(app) + .get('/') + .expect('ETag', /^W\/".+"$/) + .expect(200, 'hello', done); + }); + + it('should generate same ETag for same origin', function(done){ + var app = express(); + app.set('etag', 'weak-cors'); + + app.use(function(req, res){ + res.set('Access-Control-Allow-Origin', 'https://example.com'); + res.send('content'); + }); + + request(app) + .get('/') + .expect(200) + .end(function(err, res1){ + if (err) return done(err); + var etag1 = res1.headers.etag; + + request(app) + .get('/') + .expect(200) + .end(function(err, res2){ + if (err) return done(err); + var etag2 = res2.headers.etag; + assert.strictEqual(etag1, etag2, 'ETags should be the same for same origin'); + done(); + }); + }); + }); + }); + + describe('when etag is set to strong-cors', function(){ + it('should generate strong CORS-aware ETag', function(done){ + var app = express(); + app.set('etag', 'strong-cors'); + + app.use(function(req, res){ + res.set('Access-Control-Allow-Origin', 'https://example.com'); + res.send('hello'); + }); + + request(app) + .get('/') + .expect(function(res){ + var etag = res.headers.etag; + assert.ok(etag, 'ETag should be present'); + assert.ok(!etag.startsWith('W/'), 'ETag should be strong (no W/ prefix)'); + }) + .expect(200, 'hello', done); + }); + + it('should generate different strong ETags for different origins', function(done){ + var app = express(); + app.set('etag', 'strong-cors'); + + app.use(function(req, res){ + var origin = req.get('Origin') || 'https://default.com'; + res.set('Access-Control-Allow-Origin', origin); + res.send('body'); + }); + + request(app) + .get('/') + .set('Origin', 'https://x.com') + .expect(200) + .end(function(err, res1){ + if (err) return done(err); + var etag1 = res1.headers.etag; + + request(app) + .get('/') + .set('Origin', 'https://y.com') + .expect(200) + .end(function(err, res2){ + if (err) return done(err); + var etag2 = res2.headers.etag; + assert.notStrictEqual(etag1, etag2); + assert.ok(!etag1.startsWith('W/'), 'ETag1 should be strong'); + assert.ok(!etag2.startsWith('W/'), 'ETag2 should be strong'); + done(); + }); + }); + }); + }); + + describe('backward compatibility', function(){ + it('should not change behavior when etag is set to weak', function(done){ + var app = express(); + app.set('etag', 'weak'); + + app.use(function(req, res){ + res.set('Access-Control-Allow-Origin', 'https://example.com'); + res.send('hello'); + }); + + // With default weak mode, same body should have same ETag regardless of CORS headers + request(app) + .get('/') + .set('Origin', 'https://a.com') + .expect(200) + .end(function(err, res1){ + if (err) return done(err); + var etag1 = res1.headers.etag; + + request(app) + .get('/') + .set('Origin', 'https://b.com') + .expect(200) + .end(function(err, res2){ + if (err) return done(err); + var etag2 = res2.headers.etag; + // In non-CORS mode, ETags should be the same (body-only) + assert.strictEqual(etag1, etag2, 'ETags should be same in backward compatible mode'); + done(); + }); + }); + }); + + it('should support JSON responses with CORS-aware ETags', function(done){ + var app = express(); + app.set('etag', 'weak-cors'); + + app.use(function(req, res){ + res.set('Access-Control-Allow-Origin', 'https://example.com'); + res.json({ message: 'hello' }); + }); + + request(app) + .get('/') + .expect('Content-Type', /json/) + .expect('ETag', /^W\/".+"$/) + .expect(200, done); + }); + }); +}); diff --git a/test/utils.js b/test/utils.js index 1c06036aa98..c8ac2cc8be1 100644 --- a/test/utils.js +++ b/test/utils.js @@ -81,3 +81,91 @@ describe('utils.wetag(body, encoding)', function(){ 'W/"0-2jmj7l5rSw0yVb/vlWAYkK/YBwk"') }) }) + +describe('utils.etagCors(body, encoding, headers)', function(){ + it('should support strings without headers', function(){ + var etag1 = utils.etagCors('express!', 'utf8', {}); + var etag2 = utils.etagCors('express!', 'utf8', {}); + assert.strictEqual(etag1, etag2); + assert.strictEqual(etag1.startsWith('"'), true); + }) + + it('should generate different ETags for different origins', function(){ + var etag1 = utils.etagCors('express!', 'utf8', { + 'access-control-allow-origin': 'https://a.com' + }); + var etag2 = utils.etagCors('express!', 'utf8', { + 'access-control-allow-origin': 'https://b.com' + }); + assert.notStrictEqual(etag1, etag2); + }) + + it('should generate same ETag for same origin', function(){ + var headers = { 'access-control-allow-origin': 'https://a.com' }; + var etag1 = utils.etagCors('express!', 'utf8', headers); + var etag2 = utils.etagCors('express!', 'utf8', headers); + assert.strictEqual(etag1, etag2); + }) + + it('should work with lowercase header names (as returned by getHeaders)', function(){ + // Node.js getHeaders() always returns lowercase keys + var headers = { 'access-control-allow-origin': 'https://a.com' }; + var etag1 = utils.etagCors('express!', 'utf8', headers); + var etag2 = utils.etagCors('express!', 'utf8', headers); + assert.strictEqual(etag1, etag2); + // Verify it includes the header in the hash + var etagWithoutHeader = utils.etagCors('express!', 'utf8', {}); + assert.notStrictEqual(etag1, etagWithoutHeader); + }) + + it('should handle missing CORS headers gracefully', function(){ + var etagWithoutCORS = utils.etagCors('express!', 'utf8', { + 'content-type': 'text/plain' + }); + // Should still generate an ETag (falls back to body-only) + assert.ok(etagWithoutCORS); + assert.strictEqual(etagWithoutCORS.startsWith('"'), true); + }) + + it('should support buffer with headers', function(){ + var etag1 = utils.etagCors(Buffer.from('express!'), undefined, { + 'access-control-allow-origin': 'https://a.com' + }); + var etag2 = utils.etagCors(Buffer.from('express!'), undefined, { + 'access-control-allow-origin': 'https://b.com' + }); + assert.notStrictEqual(etag1, etag2); + }) + + it('should be backward compatible without headers parameter', function(){ + var etag = utils.etagCors('express!'); + assert.ok(etag); + assert.strictEqual(etag, '"8-O2uVAFaQ1rZvlKLT14RnuvjPIdg"'); + }) +}) + +describe('utils.wetagCors(body, encoding, headers)', function(){ + it('should generate weak ETags', function(){ + var etag = utils.wetagCors('express!', 'utf8', { + 'access-control-allow-origin': 'https://example.com' + }); + assert.ok(etag.startsWith('W/"')); + }) + + it('should generate different weak ETags for different origins', function(){ + var etag1 = utils.wetagCors('express!', 'utf8', { + 'access-control-allow-origin': 'https://a.com' + }); + var etag2 = utils.wetagCors('express!', 'utf8', { + 'access-control-allow-origin': 'https://b.com' + }); + assert.notStrictEqual(etag1, etag2); + assert.ok(etag1.startsWith('W/"')); + assert.ok(etag2.startsWith('W/"')); + }) + + it('should be backward compatible without headers', function(){ + var etag = utils.wetagCors('express!'); + assert.strictEqual(etag, 'W/"8-O2uVAFaQ1rZvlKLT14RnuvjPIdg"'); + }) +})