Skip to content
This repository was archived by the owner on Apr 10, 2025. It is now read-only.

Commit 9ba2502

Browse files
jeffkaufmancrowell
authored andcommitted
Don't set Last-Modified on IPRO Requests
Fixes #1106
1 parent 16e9f43 commit 9ba2502

File tree

6 files changed

+39
-20
lines changed

6 files changed

+39
-20
lines changed

net/instaweb/apache/apache_writer.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ void ApacheWriter::OutputHeaders(ResponseHeaders* response_headers) {
8080
if (content_length_ != AsyncFetch::kContentLengthUnknown) {
8181
ap_set_content_length(request_, content_length_);
8282
}
83-
8483
ResponseHeadersToApacheRequest(*response_headers, request_);
8584
request_->status = response_headers->status_code();
8685
if (disable_downstream_header_filters_) {

net/instaweb/apache/system_test.sh

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,28 @@ if [ $statistics_logging_enabled = "1" ]; then
128128
sleep 2; # Make sure we're around long enough to log stats.
129129
fi
130130

131-
# General system tests
131+
start_test ipro resources have etag and not last-modified
132+
URL="$EXAMPLE_ROOT/images/Puzzle.jpg?a=$RANDOM"
133+
# Fetch it a few times until IPRO is done and has given it an ipro ("aj") etag.
134+
fetch_until -save "$URL" 'grep -c E[Tt]ag:.W/.PSA-aj.' 1 --save-headers
135+
# Verify that it doesn't have a Last-Modified header.
136+
check [ $(grep -c "^Last-Modified:" $FETCH_FILE) = 0 ]
137+
# Extract the Etag and verify we get "not modified" when we send the it.
138+
ETAG=$(grep -a -o 'W/"PSA-aj[^"]*"' $FETCH_FILE)
139+
check_from "$ETAG" fgrep PSA
140+
echo $CURL -sS -D- -o/dev/null -H "If-None-Match: $ETAG" $URL
141+
OUT=$($CURL -sS -D- -o/dev/null -H "If-None-Match: $ETAG" $URL)
142+
check_from "$OUT" fgrep "HTTP/1.1 304"
143+
check_not_from "$OUT" fgrep "Content-Length"
144+
# Verify we don't get a 304 with a different Etag.
145+
BAD_ETAG=$(echo "$ETAG" | sed s/PSA-aj/PSA-ic/)
146+
echo $CURL -sS -D- -o/dev/null -H "If-None-Match: $BAD_ETAG" $URL
147+
OUT=$($CURL -sS -D- -o/dev/null -H "If-None-Match: $BAD_ETAG" $URL)
148+
check_not_from "$OUT" fgrep "HTTP/1.1 304"
149+
check_from "$OUT" fgrep "HTTP/1.1 200"
150+
check_from "$OUT" fgrep "Content-Length"
132151

152+
# General system tests
133153
start_test Check for correct default X-Mod-Pagespeed header format.
134154
OUT=$($WGET_DUMP $EXAMPLE_ROOT/combine_css.html)
135155
check_from "$OUT" egrep -q \

net/instaweb/rewriter/in_place_rewrite_context.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ void InPlaceRewriteContext::FixFetchFallbackHeaders(
389389
headers->Replace(HttpAttributes::kEtag, HTTPCache::FormatEtag(StrCat(
390390
id(), "-", rewritten_hash_)));
391391
}
392+
headers->RemoveAll(HttpAttributes::kLastModified);
392393
headers->set_implicit_cache_ttl_ms(Options()->implicit_cache_ttl_ms());
393394
headers->set_min_cache_ttl_ms(Options()->min_cache_ttl_ms());
394395
headers->ComputeCaching();

net/instaweb/system/admin_site.cc

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,16 @@ const Tab kTabs[] = {
103103
class AdminHtml {
104104
public:
105105
AdminHtml(StringPiece current_link, StringPiece head_extra,
106-
AdminSite::AdminSource source, AsyncFetch* fetch,
107-
MessageHandler* handler)
106+
AdminSite::AdminSource source, Timer* timer,
107+
AsyncFetch* fetch, MessageHandler* handler)
108108
: fetch_(fetch),
109109
handler_(handler) {
110110
fetch->response_headers()->SetStatusAndReason(HttpStatus::kOK);
111111
fetch->response_headers()->Add(HttpAttributes::kContentType, "text/html");
112112

113+
int64 now_ms = timer->NowMs();
114+
fetch->response_headers()->SetLastModified(now_ms);
115+
113116
// Let PageSpeed dynamically minify the html, css, and javasript
114117
// generated by the admin page, to the extent it's not done
115118
// already by the tools. Note, this does mean that viewing the
@@ -211,7 +214,7 @@ void AdminSite::ConsoleHandler(const SystemRewriteOptions& global_options,
211214
// TODO(sligocki): Do we want to have a minified version of console CSS?
212215
GoogleString head_markup = StrCat(
213216
"<style>", CSS_console_css, "</style>\n");
214-
AdminHtml admin_html("console", head_markup, source, fetch,
217+
AdminHtml admin_html("console", head_markup, source, timer_, fetch,
215218
message_handler_);
216219
if (statistics_enabled && logging_enabled && log_dir_set) {
217220
fetch->Write("<div class='console_div' id='suggestions'>\n"
@@ -267,7 +270,7 @@ void AdminSite::StatisticsHandler(const RewriteOptions& options,
267270
Statistics* stats) {
268271
GoogleString head_markup = StrCat(
269272
"<style>", CSS_statistics_css, "</style>\n");
270-
AdminHtml admin_html("statistics", head_markup, source, fetch,
273+
AdminHtml admin_html("statistics", head_markup, source, timer_, fetch,
271274
message_handler_);
272275
// We have to call Dump() here to write data to sources for
273276
// system/system_test.sh: Line 79. We use JS to update the data in refreshes.
@@ -293,7 +296,8 @@ void AdminSite::GraphsHandler(const RewriteOptions& options,
293296
}
294297
GoogleString head_markup = StrCat(
295298
"<style>", CSS_graphs_css, "</style>\n");
296-
AdminHtml admin_html("graphs", head_markup, source, fetch, message_handler_);
299+
AdminHtml admin_html("graphs", head_markup, source, timer_, fetch,
300+
message_handler_);
297301
fetch->Write("<div id='cache_applied'>Loading Charts...</div>"
298302
"<div id='cache_type'>Loading Charts...</div>"
299303
"<div id='ipro'>Loading Charts...</div>"
@@ -367,7 +371,8 @@ void AdminSite::ConsoleJsonHandler(const QueryParams& params,
367371

368372
void AdminSite::PrintHistograms(AdminSource source, AsyncFetch* fetch,
369373
Statistics* stats) {
370-
AdminHtml admin_html("histograms", "", source, fetch, message_handler_);
374+
AdminHtml admin_html("histograms", "", source, timer_, fetch,
375+
message_handler_);
371376
stats->RenderHistograms(fetch, message_handler_);
372377
}
373378

@@ -551,7 +556,7 @@ void AdminSite::PrintCaches(bool is_global, AdminSource source,
551556
} else {
552557
GoogleString head_markup = StrCat(
553558
"<style>", CSS_caches_css, "</style>\n");
554-
AdminHtml admin_html("cache", head_markup, source, fetch,
559+
AdminHtml admin_html("cache", head_markup, source, timer_, fetch,
555560
message_handler_);
556561

557562
fetch->Write("<div id='show_metadata'>", message_handler_);
@@ -624,15 +629,16 @@ void AdminSite::PrintCaches(bool is_global, AdminSource source,
624629
void AdminSite::PrintNormalConfig(
625630
AdminSource source, AsyncFetch* fetch,
626631
SystemRewriteOptions* global_system_rewrite_options) {
627-
AdminHtml admin_html("config", "", source, fetch, message_handler_);
632+
AdminHtml admin_html("config", "", source, timer_, fetch, message_handler_);
628633
HtmlKeywords::WritePre(
629634
global_system_rewrite_options->OptionsToString(), "",
630635
fetch, message_handler_);
631636
}
632637

633638
void AdminSite::PrintSpdyConfig(AdminSource source, AsyncFetch* fetch,
634639
const SystemRewriteOptions* spdy_config) {
635-
AdminHtml admin_html("spdy_config", "", source, fetch, message_handler_);
640+
AdminHtml admin_html("spdy_config", "", source, timer_, fetch,
641+
message_handler_);
636642
if (spdy_config == NULL) {
637643
fetch->Write("SPDY-specific configuration missing.", message_handler_);
638644
} else {
@@ -646,7 +652,8 @@ void AdminSite::MessageHistoryHandler(const RewriteOptions& options,
646652
// Request for page /mod_pagespeed_message.
647653
GoogleString log;
648654
StringWriter log_writer(&log);
649-
AdminHtml admin_html("message_history", "", source, fetch, message_handler_);
655+
AdminHtml admin_html("message_history", "", source, timer_, fetch,
656+
message_handler_);
650657
if (message_handler_->Dump(&log_writer)) {
651658
fetch->Write("<div id='log'>", message_handler_);
652659
// Write pre-tag and color messages.

pagespeed/apache/apache_fetch.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ void ApacheFetch::HandleHeadersComplete() {
8787

8888
int64 now_ms = timer_->NowMs();
8989
response_headers()->SetDate(now_ms);
90-
response_headers()->SetLastModified(now_ms);
9190

9291
// http://msdn.microsoft.com/en-us/library/ie/gg622941(v=vs.85).aspx
9392
// Script and styleSheet elements will reject responses with

pagespeed/apache/apache_fetch_test.cc

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,6 @@ TEST_F(ApacheFetchTest, Success) {
213213
"Set-Cookie: tasty=cookie\n"
214214
"Set-Cookie2: obselete\n"
215215
"Date: Thu, 01 Jan 1970 00:00:00 GMT\n"
216-
"Last-Modified: Thu, 01 Jan 1970 00:00:00 GMT\n"
217216
"X-Content-Type-Options: nosniff\n"
218217
"Cache-Control: max-age=0, no-cache\n",
219218
HeadersOutToString(&request_));
@@ -239,7 +238,6 @@ TEST_F(ApacheFetchTest, NotFound404) {
239238
"Set-Cookie: tasty=cookie\n"
240239
"Set-Cookie2: obselete\n"
241240
"Date: Thu, 01 Jan 1970 00:00:00 GMT\n"
242-
"Last-Modified: Thu, 01 Jan 1970 00:00:00 GMT\n"
243241
"X-Content-Type-Options: nosniff\n"
244242
"Cache-Control: max-age=0, no-cache\n",
245243
HeadersOutToString(&request_));
@@ -264,7 +262,6 @@ TEST_F(ApacheFetchTest, NoContentType200) {
264262
"Set-Cookie2: obselete\n"
265263
"Content-Type: text/html\n"
266264
"Date: Thu, 01 Jan 1970 00:00:00 GMT\n"
267-
"Last-Modified: Thu, 01 Jan 1970 00:00:00 GMT\n"
268265
"X-Content-Type-Options: nosniff\n"
269266
"Cache-Control: max-age=0, no-cache\n",
270267
HeadersOutToString(&request_));
@@ -290,7 +287,6 @@ TEST_F(ApacheFetchTest, NoContentType301) {
290287
"Location: elsewhere\n"
291288
"Content-Type: text/html\n"
292289
"Date: Thu, 01 Jan 1970 00:00:00 GMT\n"
293-
"Last-Modified: Thu, 01 Jan 1970 00:00:00 GMT\n"
294290
"X-Content-Type-Options: nosniff\n"
295291
"Cache-Control: max-age=0, no-cache\n",
296292
HeadersOutToString(&request_));
@@ -313,7 +309,6 @@ TEST_F(ApacheFetchTest, NoContentType304) {
313309
"Set-Cookie: tasty=cookie\n"
314310
"Set-Cookie2: obselete\n"
315311
"Date: Thu, 01 Jan 1970 00:00:00 GMT\n"
316-
"Last-Modified: Thu, 01 Jan 1970 00:00:00 GMT\n"
317312
"X-Content-Type-Options: nosniff\n"
318313
"Cache-Control: max-age=0, no-cache\n",
319314
HeadersOutToString(&request_));
@@ -334,7 +329,6 @@ TEST_F(ApacheFetchTest, NoContentType204) {
334329
"Set-Cookie: tasty=cookie\n"
335330
"Set-Cookie2: obselete\n"
336331
"Date: Thu, 01 Jan 1970 00:00:00 GMT\n"
337-
"Last-Modified: Thu, 01 Jan 1970 00:00:00 GMT\n"
338332
"X-Content-Type-Options: nosniff\n"
339333
"Cache-Control: max-age=0, no-cache\n",
340334
HeadersOutToString(&request_));
@@ -361,7 +355,6 @@ TEST_F(ApacheFetchTest, AbandonedAndHandled) {
361355
"Set-Cookie: tasty=cookie\n"
362356
"Set-Cookie2: obselete\n"
363357
"Date: Thu, 01 Jan 1970 00:00:00 GMT\n"
364-
"Last-Modified: Thu, 01 Jan 1970 00:00:00 GMT\n"
365358
"X-Content-Type-Options: nosniff\n"
366359
"Cache-Control: max-age=0, no-cache\n",
367360
HeadersOutToString(&request_));

0 commit comments

Comments
 (0)