Skip to content

Commit ae6dd37

Browse files
Cherrybjohansebas
andauthored
feat: allow conditional revalidation for QUERY requests (#7366)
* feat: allow conditional revalidation for QUERY requests req.fresh only validated freshness for GET and HEAD, so QUERY responses never returned 304 despite a matching If-None-Match. QUERY is a safe, idempotent, cacheable method that supports conditional requests, so include it in the freshness check. * docs: include history --------- Co-authored-by: Sebastian Beltran <bjohansebas@gmail.com>
1 parent ba00676 commit ae6dd37

3 files changed

Lines changed: 50 additions & 2 deletions

File tree

History.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,16 @@
99

1010
## 🚀 Improvements
1111

12+
* Allow conditional revalidation for QUERY requests. `req.fresh` previously only validated freshness for GET and HEAD requests, so QUERY responses never returned 304 despite a matching validator. Since QUERY is a safe, idempotent, and cacheable method that supports conditional requests, it is now included in the freshness check - by [@Cherry](https://github.com/Cherry) in [#7366](https://github.com/expressjs/express/pull/7366)
13+
14+
```js
15+
// QUERY /reports with If-None-Match: "12345"
16+
app.query('/reports', (req, res) => {
17+
res.set('ETag', '"12345"');
18+
res.send(results); // now responds 304 Not Modified
19+
});
20+
```
21+
1222
* Improve HTML structure in `res.redirect()` responses when HTML format is accepted by adding `<!DOCTYPE html>`, `<title>`, and `<body>` tags for better browser compatibility - by [@Bernice55231](https://github.com/Bernice55231) in [#5167](https://github.com/expressjs/express/pull/5167)
1323

1424
* When calling `app.render` with options set to null, the locals object is handled correctly, preventing unexpected errors and making the method behave the same as when options is omitted or an empty object is passed - by [AkaHarshit](https://github.com/AkaHarshit) in [#6903](https://github.com/expressjs/express/pull/6903)

lib/request.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -471,8 +471,8 @@ defineGetter(req, 'fresh', function(){
471471
var res = this.res
472472
var status = res.statusCode
473473

474-
// GET or HEAD for weak freshness validation only
475-
if ('GET' !== method && 'HEAD' !== method) return false;
474+
// GET, HEAD, or QUERY for weak freshness validation only
475+
if ('GET' !== method && 'HEAD' !== method && 'QUERY' !== method) return false;
476476

477477
// 2xx or 304 as per rfc2616 14.26
478478
if ((status >= 200 && status < 300) || 304 === status) {

test/req.fresh.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
var express = require('../')
44
, request = require('supertest');
5+
var shouldSkipQuery = require('./support/utils').shouldSkipQuery
56

67
describe('req', function(){
78
describe('.fresh', function(){
@@ -47,6 +48,43 @@ describe('req', function(){
4748
.expect(200, 'false', done);
4849
})
4950

51+
it('should return true for a QUERY request with a body when the resource is not modified', function(done){
52+
if (shouldSkipQuery(process.versions.node)) {
53+
this.skip()
54+
}
55+
var app = express();
56+
var etag = '"12345"';
57+
58+
app.use(function(req, res){
59+
res.set('ETag', etag);
60+
res.send(req.fresh);
61+
});
62+
63+
request(app)
64+
.query('/')
65+
.set('If-None-Match', etag)
66+
.send({ ids: ['a', 'b'] })
67+
.expect(304, done);
68+
})
69+
70+
it('should return false for a QUERY request with a body when the resource is modified', function(done){
71+
if (shouldSkipQuery(process.versions.node)) {
72+
this.skip()
73+
}
74+
var app = express();
75+
76+
app.use(function(req, res){
77+
res.set('ETag', '"123"');
78+
res.send(req.fresh);
79+
});
80+
81+
request(app)
82+
.query('/')
83+
.set('If-None-Match', '"12345"')
84+
.send({ ids: ['a', 'b'] })
85+
.expect(200, 'false', done);
86+
})
87+
5088
it('should ignore "If-Modified-Since" when "If-None-Match" is present', function(done) {
5189
var app = express();
5290
const etag = '"FooBar"'

0 commit comments

Comments
 (0)