Skip to content

Commit c521eb0

Browse files
bneradtbneradt
committed
Fix s-maxage not respected with Authorization headers
According to RFC 7234 section 3.2, a shared cache can serve cached responses to requests with Authorization headers if the response contains one of the following Cache-Control directives: must-revalidate, proxy-revalidate, public, or s-maxage. The implementation was missing the check for s-maxage, causing requests with Authorization headers to always bypass the cache even when s-maxage was present in the cached response. This commit adds the missing check for s-maxage in the AuthenticationNeeded() function and includes comprehensive tests to verify the fix and prevent regression. Fixes #7227
1 parent d74c795 commit c521eb0

File tree

3 files changed

+215
-5
lines changed

3 files changed

+215
-5
lines changed

‎src/proxy/http/HttpTransact.cc‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7486,11 +7486,11 @@ HttpTransact::what_is_document_freshness(State *s, HTTPHdr *client_request, HTTP
74867486
HttpTransact::Authentication_t
74877487
HttpTransact::AuthenticationNeeded(const OverridableHttpConfigParams *p, HTTPHdr *client_request, HTTPHdr *obj_response)
74887488
{
7489-
///////////////////////////////////////////////////////////////////////
7490-
// from RFC2068, sec 14.8, if a client request has the Authorization //
7491-
// header set, we can't serve it unless the response is public, or //
7492-
// if it has a Cache-Control revalidate flag, and we do revalidate. //
7493-
///////////////////////////////////////////////////////////////////////
7489+
///////////////////////////////////////////////////////////////////////////////
7490+
// Per RFC 7234 section 3.2, if a client request has the Authorization //
7491+
// header set, we can't serve a cached response unless the response has one //
7492+
// of: must-revalidate, proxy-revalidate, public, or s-maxage directives. //
7493+
///////////////////////////////////////////////////////////////////////////////
74947494

74957495
if ((p->cache_ignore_auth == 0) && client_request->presence(MIME_PRESENCE_AUTHORIZATION)) {
74967496
if (obj_response->is_cache_control_set(HTTP_VALUE_MUST_REVALIDATE.c_str()) ||
@@ -7500,6 +7500,8 @@ HttpTransact::AuthenticationNeeded(const OverridableHttpConfigParams *p, HTTPHdr
75007500
return Authentication_t::MUST_REVALIDATE;
75017501
} else if (obj_response->is_cache_control_set(HTTP_VALUE_PUBLIC.c_str())) {
75027502
return Authentication_t::SUCCESS;
7503+
} else if (obj_response->is_cache_control_set(HTTP_VALUE_S_MAXAGE.c_str())) {
7504+
return Authentication_t::SUCCESS;
75037505
} else {
75047506
if (obj_response->field_find("@WWW-Auth"sv) && client_request->method_get_wksidx() == HTTP_WKSIDX_GET) {
75057507
return Authentication_t::CACHE_AUTH;

‎tests/gold_tests/cache/cache-auth.test.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,7 @@
2323

2424
# Verify proxy.config.http.cache.ignore_authentication behavior.
2525
Test.ATSReplayTest(replay_file="replay/ignore_authentication.replay.yaml")
26+
27+
# Verify that s-maxage allows serving cached responses to requests with
28+
# Authorization headers per RFC 7234 section 3.2
29+
Test.ATSReplayTest(replay_file="replay/auth-s-maxage.replay.yaml")

‎tests/gold_tests/cache/replay/auth-s-maxage.replay.yaml‎

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
#
18+
# This replay file tests RFC 7234 section 3.2:
19+
# A shared cache can serve cached responses to requests with Authorization
20+
# headers if the response has s-maxage directive (along with must-revalidate
21+
# and public).
22+
#
23+
# This replay file assumes that caching is enabled and
24+
# proxy.config.http.cache.ignore_authentication is set to 0 (current default).
25+
#
26+
meta:
27+
version: "1.0"
28+
29+
# Configuration section for autest integration
30+
autest:
31+
description: 'Test s-maxage allows serving cached responses to requests with Authorization headers per RFC 7234 section 3.2'
32+
33+
# Server configuration
34+
server:
35+
name: 'server-s-maxage'
36+
37+
# Client configuration
38+
client:
39+
name: 'client-s-maxage'
40+
41+
# ATS configuration
42+
ats:
43+
name: 'ts-auth-s-maxage'
44+
45+
# ATS process configuration
46+
process_config:
47+
enable_cache: true
48+
49+
# ATS records.config settings
50+
records_config:
51+
proxy.config.diags.debug.enabled: 1
52+
proxy.config.diags.debug.tags: 'http'
53+
54+
# Remap configuration
55+
remap_config:
56+
- from: "/"
57+
to: "http://127.0.0.1:{SERVER_HTTP_PORT}/"
58+
59+
sessions:
60+
- transactions:
61+
# First request: Cache a 200 response with s-maxage.
62+
- client-request:
63+
method: "GET"
64+
version: "1.1"
65+
url: /auth/s-maxage-test
66+
headers:
67+
fields:
68+
- [uuid, s-maxage-cache-fill]
69+
- [Host, example.com]
70+
71+
server-response:
72+
status: 200
73+
reason: OK
74+
headers:
75+
fields:
76+
- [Content-Length, 16]
77+
- [Cache-Control, "s-maxage=300"]
78+
79+
proxy-response:
80+
status: 200
81+
82+
# Second request: Request with Authorization header should be served from cache
83+
# because response has s-maxage per RFC 7234 section 3.2.
84+
- client-request:
85+
# Add a delay so ATS has time to finish any caching IO for the
86+
# previous transaction.
87+
delay: 100ms
88+
method: "GET"
89+
version: "1.1"
90+
url: /auth/s-maxage-test
91+
headers:
92+
fields:
93+
- [uuid, s-maxage-with-auth]
94+
- [Host, example.com]
95+
- [Authorization, "Basic ZGVtbzphdHNAc3Ryb25ncHc="]
96+
97+
# The server should NOT be reached because the cached response
98+
# with s-maxage should be served. Return 400 to verify caching.
99+
server-response:
100+
status: 400
101+
reason: "Bad Request"
102+
headers:
103+
fields:
104+
- [Content-Length, 0]
105+
106+
# Expect the cached 200 response per RFC 7234 section 3.2
107+
proxy-response:
108+
status: 200
109+
110+
- transactions:
111+
# Test 2: Verify that public also works.
112+
- client-request:
113+
method: "GET"
114+
version: "1.1"
115+
url: /auth/public-test
116+
headers:
117+
fields:
118+
- [uuid, public-cache-fill]
119+
- [Host, example.com]
120+
121+
server-response:
122+
status: 200
123+
reason: OK
124+
headers:
125+
fields:
126+
- [Content-Length, 16]
127+
- [Cache-Control, "public, max-age=300"]
128+
129+
proxy-response:
130+
status: 200
131+
132+
# Request with Authorization header should be served from cache with public
133+
- client-request:
134+
delay: 100ms
135+
method: "GET"
136+
version: "1.1"
137+
url: /auth/public-test
138+
headers:
139+
fields:
140+
- [uuid, public-with-auth]
141+
- [Host, example.com]
142+
- [Authorization, "Basic ZGVtbzphdHNAc3Ryb25ncHc="]
143+
144+
# The server should NOT be reached because the cached response
145+
# with public should be served. Return 400 to verify caching.
146+
server-response:
147+
status: 400
148+
reason: "Bad Request"
149+
headers:
150+
fields:
151+
- [Content-Length, 0]
152+
153+
# Expect cached 200 response
154+
proxy-response:
155+
status: 200
156+
157+
- transactions:
158+
# Test 3: Without s-maxage or public, Authorization should require revalidation
159+
- client-request:
160+
method: "GET"
161+
version: "1.1"
162+
url: /auth/no-directive-test
163+
headers:
164+
fields:
165+
- [uuid, no-directive-cache-fill]
166+
- [Host, example.com]
167+
168+
server-response:
169+
status: 200
170+
reason: OK
171+
headers:
172+
fields:
173+
- [Content-Length, 16]
174+
- [Cache-Control, "max-age=300"]
175+
176+
proxy-response:
177+
status: 200
178+
179+
# Request with Authorization header should NOT be served from cache
180+
- client-request:
181+
delay: 100ms
182+
method: "GET"
183+
version: "1.1"
184+
url: /auth/no-directive-test
185+
headers:
186+
fields:
187+
- [uuid, no-directive-with-auth]
188+
- [Host, example.com]
189+
- [Authorization, "Basic ZGVtbzphdHNAc3Ryb25ncHc="]
190+
191+
# The request should be forwarded to the server and a 404 response
192+
# should be returned.
193+
server-response:
194+
status: 404
195+
reason: "Not Found"
196+
headers:
197+
fields:
198+
- [Content-Length, 20]
199+
200+
# Expect the new 404 response from server rather than the previous 200
201+
# response.
202+
proxy-response:
203+
status: 404
204+

0 commit comments

Comments
 (0)