Skip to content

Commit 2cf6420

Browse files
agentzhagentzh
committed
bugfix: we used to skip all the output header and body filters run before our filters (which unfortunately bypassed the standard ngx_not_modified module, for example). thanks Lloyd Zhou for the report in openresty#29.
1 parent de85550 commit 2cf6420

File tree

2 files changed

+113
-104
lines changed

2 files changed

+113
-104
lines changed

‎src/ngx_http_srcache_fetch.c

Lines changed: 3 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,6 @@
1818

1919
static ngx_int_t ngx_http_srcache_fetch_subrequest(ngx_http_request_t *r,
2020
ngx_http_srcache_loc_conf_t *conf, ngx_http_srcache_ctx_t *ctx);
21-
static ngx_int_t ngx_http_srcache_fetch_header_filter(ngx_http_request_t *r);
22-
static ngx_int_t ngx_http_srcache_test_not_modified(ngx_http_request_t *r);
23-
#if defined(nginx_version) && (nginx_version >= 9002)
24-
static ngx_int_t ngx_http_srcache_test_precondition(ngx_http_request_t *r);
25-
#endif
2621
static void ngx_http_srcache_post_read_body(ngx_http_request_t *r);
2722

2823

@@ -160,7 +155,7 @@ ngx_http_srcache_access_handler(ngx_http_request_t *r)
160155

161156
r->headers_out.content_length_n = len;
162157

163-
rc = ngx_http_srcache_fetch_header_filter(r);
158+
rc = ngx_http_send_header(r);
164159

165160
dd("srcache fetch header returned %d", (int) rc);
166161

@@ -175,15 +170,8 @@ ngx_http_srcache_access_handler(ngx_http_request_t *r)
175170
#endif
176171

177172
if (!r->filter_finalize) {
178-
rc = ngx_http_srcache_next_body_filter(r,
179-
ctx->body_from_cache);
180-
181-
if (rc == NGX_ERROR) {
182-
r->connection->error = 1;
183-
return NGX_ERROR;
184-
}
185-
186-
if (rc > NGX_OK) {
173+
rc = ngx_http_output_filter(r, ctx->body_from_cache);
174+
if (rc == NGX_ERROR || rc > NGX_OK) {
187175
return rc;
188176
}
189177
}
@@ -446,95 +434,6 @@ ngx_http_srcache_fetch_subrequest(ngx_http_request_t *r,
446434
}
447435

448436

449-
static ngx_int_t
450-
ngx_http_srcache_fetch_header_filter(ngx_http_request_t *r)
451-
{
452-
if (r->headers_out.status != NGX_HTTP_OK
453-
|| r->headers_out.last_modified_time == -1)
454-
{
455-
return ngx_http_srcache_next_header_filter(r);
456-
}
457-
458-
#if defined(nginx_version) && (nginx_version >= 9002)
459-
if (r->headers_in.if_unmodified_since) {
460-
return ngx_http_srcache_test_precondition(r);
461-
}
462-
#endif
463-
464-
if (r->headers_in.if_modified_since) {
465-
return ngx_http_srcache_test_not_modified(r);
466-
}
467-
468-
return ngx_http_srcache_next_header_filter(r);
469-
}
470-
471-
472-
static ngx_int_t
473-
ngx_http_srcache_test_not_modified(ngx_http_request_t *r)
474-
{
475-
time_t ims;
476-
ngx_http_core_loc_conf_t *clcf;
477-
478-
clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
479-
480-
if (clcf->if_modified_since == NGX_HTTP_IMS_OFF) {
481-
return ngx_http_srcache_next_header_filter(r);
482-
}
483-
484-
ims = ngx_http_parse_time(r->headers_in.if_modified_since->value.data,
485-
r->headers_in.if_modified_since->value.len);
486-
487-
ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
488-
"srcache ims:%d lm:%d", ims,
489-
r->headers_out.last_modified_time);
490-
491-
if (ims != r->headers_out.last_modified_time) {
492-
493-
if (clcf->if_modified_since == NGX_HTTP_IMS_EXACT
494-
|| ims < r->headers_out.last_modified_time)
495-
{
496-
return ngx_http_srcache_next_header_filter(r);
497-
}
498-
}
499-
500-
r->headers_out.status = NGX_HTTP_NOT_MODIFIED;
501-
r->headers_out.status_line.len = 0;
502-
r->headers_out.content_type.len = 0;
503-
ngx_http_clear_content_length(r);
504-
ngx_http_clear_accept_ranges(r);
505-
506-
if (r->headers_out.content_encoding) {
507-
r->headers_out.content_encoding->hash = 0;
508-
r->headers_out.content_encoding = NULL;
509-
}
510-
511-
return ngx_http_srcache_next_header_filter(r);
512-
}
513-
514-
515-
#if defined(nginx_version) && (nginx_version >= 9002)
516-
static ngx_int_t
517-
ngx_http_srcache_test_precondition(ngx_http_request_t *r)
518-
{
519-
time_t iums;
520-
521-
iums = ngx_http_parse_time(r->headers_in.if_unmodified_since->value.data,
522-
r->headers_in.if_unmodified_since->value.len);
523-
524-
ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
525-
"srcache iums:%d lm:%d", iums,
526-
r->headers_out.last_modified_time);
527-
528-
if (iums >= r->headers_out.last_modified_time) {
529-
return ngx_http_srcache_next_header_filter(r);
530-
}
531-
532-
return ngx_http_filter_finalize_request(r, NULL,
533-
NGX_HTTP_PRECONDITION_FAILED);
534-
}
535-
#endif
536-
537-
538437
static void
539438
ngx_http_srcache_post_read_body(ngx_http_request_t *r)
540439
{

‎t/etag.t

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# vi:filetype=
2+
3+
use lib 'lib';
4+
use Test::Nginx::Socket;
5+
6+
#repeat_each(2);
7+
8+
plan tests => repeat_each() * (5 * blocks() + 5);
9+
10+
$ENV{TEST_NGINX_MEMCACHED_PORT} ||= 11211;
11+
12+
#master_on();
13+
no_shuffle();
14+
15+
run_tests();
16+
17+
__DATA__
18+
19+
=== TEST 1: flush all (not using ngx_srcache)
20+
--- config
21+
location /flush {
22+
set $memc_cmd 'flush_all';
23+
memc_pass 127.0.0.1:$TEST_NGINX_MEMCACHED_PORT;
24+
add_header X-Fetch-Status $srcache_fetch_status;
25+
add_header X-Store-Status $srcache_store_status;
26+
}
27+
--- response_headers
28+
Content-Type: text/plain
29+
Content-Length: 4
30+
X-Fetch-Status: BYPASS
31+
X-Store-Status: BYPASS
32+
33+
--- request
34+
GET /flush
35+
--- response_body eval: "OK\r\n"
36+
37+
38+
39+
=== TEST 2: basic fetch (cache miss)
40+
--- config
41+
location /foo {
42+
default_type text/css;
43+
srcache_fetch GET /memc $uri;
44+
srcache_store PUT /memc $uri;
45+
46+
content_by_lua '
47+
ngx.header["ETag"] = [["abcd1234"]]
48+
ngx.header["Last-Modified"] = "Sat, 01 Mar 2014 01:02:38 GMT"
49+
ngx.say("hello")
50+
';
51+
#echo hello;
52+
add_header X-Fetch-Status $srcache_fetch_status;
53+
add_header X-Store-Status $srcache_store_status;
54+
}
55+
56+
location /memc {
57+
internal;
58+
59+
set $memc_key $query_string;
60+
set $memc_exptime 300;
61+
memc_pass 127.0.0.1:$TEST_NGINX_MEMCACHED_PORT;
62+
}
63+
--- request
64+
GET /foo
65+
--- response_headers
66+
Content-Type: text/css
67+
Content-Length:
68+
X-Fetch-Status: MISS
69+
X-Store-Status: STORE
70+
Last-Modified: Sat, 01 Mar 2014 01:02:38 GMT
71+
--- response_body
72+
hello
73+
--- wait: 0.1
74+
--- error_log
75+
srcache_store: subrequest returned status 201
76+
--- log_level: debug
77+
78+
79+
80+
=== TEST 3: basic fetch (cache hit)
81+
--- config
82+
location /foo {
83+
default_type text/css;
84+
srcache_fetch GET /memc $uri;
85+
srcache_store PUT /memc $uri;
86+
87+
echo world;
88+
add_header X-Fetch-Status $srcache_fetch_status;
89+
add_header X-Store-Status $srcache_store_status;
90+
}
91+
92+
location /memc {
93+
internal;
94+
95+
set $memc_key $query_string;
96+
set $memc_exptime 300;
97+
memc_pass 127.0.0.1:$TEST_NGINX_MEMCACHED_PORT;
98+
}
99+
--- request
100+
GET /foo
101+
--- more_headers
102+
If-None-Match: "abcd1234"
103+
--- response_headers
104+
X-Fetch-Status: HIT
105+
X-Store-Status: BYPASS
106+
ETag: "abcd1234"
107+
Last-Modified: Sat, 01 Mar 2014 01:02:38 GMT
108+
--- response_body:
109+
--- error_code: 304
110+

0 commit comments

Comments
 (0)