Drop the Last-Modified, Expires and ETag headers

10 files changed, 16 insertions, 43 deletions

diff --git a/README.txt b/README.txt
index b1ff14a..8d12dc6 100644
--- a/README.txt
+++ b/README.txt
@@ -148,4 +148,4 @@ stale cache file is returned to the client. This is done to favour page
 throughput over page freshness.
 
 The generated content contains the complete response to the client, including
-the HTTP headers `Modified` and `Expires`.
+the HTTP headers.
diff --git a/custom/servers/apache.conf b/custom/servers/apache.conf
index 7ff3bab..4345a9e 100644
--- a/custom/servers/apache.conf
+++ b/custom/servers/apache.conf
@@ -135,7 +135,11 @@ AddType text/plain               .txt
     AllowOverride None
     Require all granted
 
-    # These assets rarely change, so let browsers cache them.
+    # These assets rarely change, so let browsers cache them. Keep the
+    # caching scoped to this directory. cgit sends no caching headers of its
+    # own, so a server-wide ExpiresDefault would stamp freshness onto its
+    # pages, fight the no-store cgit puts on the login page, and could let
+    # one visitor's page be served to another from a shared cache.
     <IfModule mod_expires.c>
         ExpiresActive On
         ExpiresDefault "access plus 30 days"
diff --git a/custom/servers/nginx.conf b/custom/servers/nginx.conf
index 3b0b7ef..12d6888 100644
--- a/custom/servers/nginx.conf
+++ b/custom/servers/nginx.conf
@@ -234,6 +234,12 @@ http {
             # can take a while, so give cgit room and stream rather than buffer.
             fastcgi_read_timeout 300s;
             fastcgi_buffering off;
+
+            # cgit sends no caching headers of its own, so browsers refetch
+            # dynamic pages and cgit's internal cache keeps that cheap. Do not
+            # add a blanket expires or Cache-Control in this location. It
+            # would fight the no-store cgit puts on the login page and could
+            # let one visitor's page be served to another from a shared cache.
         }
     }
 }
diff --git a/source/cgit.c b/source/cgit.c
index 53a33ca..ac8922b 100644
--- a/source/cgit.c
+++ b/source/cgit.c
@@ -46,10 +46,6 @@
 // a snapshots mask can carry.
 #define ALL_SNAPSHOT_FORMATS 0xFF
 
-// An Expires header has no way of saying never, so a page whose ttl is
-// negative claims ten years.
-#define NEVER_EXPIRES_SECONDS (10 * 365 * 24 * 60 * 60)
-
 /*
  * The first branch found is the fallback, so a repository whose default branch
  * does not exist still has something to show.
@@ -165,9 +161,6 @@ static void prepare_context(void)
 	ctx.page.charset = PAGE_ENCODING;
 	ctx.page.filename = NULL;
 	ctx.page.size = 0;
-	ctx.page.modified = time(NULL);
-	ctx.page.expires = ctx.page.modified;
-	ctx.page.etag = NULL;
 	string_list_init_dup(&ctx.cfg.mimetypes);
 	if (ctx.env.script_name)
 		ctx.cfg.script_name = xstrdup(ctx.env.script_name);
@@ -1218,10 +1211,6 @@ int cmd_main(int argc, const char **argv)
 	authenticate_cookie();
 
 	ttl = calc_ttl();
-	if (ttl < 0)
-		ctx.page.expires += NEVER_EXPIRES_SECONDS;
-	else
-		ctx.page.expires += ttl * 60;
 	// An unauthenticated request gets a body meant for one visitor, and a
 	// HEAD request stops after the headers.
 	if (!ctx.env.authenticated ||
diff --git a/source/cgit.h b/source/cgit.h
index ed966ae..d4b19f5 100644
--- a/source/cgit.h
+++ b/source/cgit.h
@@ -268,13 +268,10 @@ struct cgit_config {
 };
 
 struct cgit_page {
-	time_t modified;
-	time_t expires;
 	size_t size;
 	const char *mimetype;
 	const char *charset;
 	const char *filename;
-	const char *etag;
 	const char *title;
 	int status;
 	const char *statusmsg;
diff --git a/source/ui-plain.c b/source/ui-plain.c
index ed1ab31..5ba650b 100644
--- a/source/ui-plain.c
+++ b/source/ui-plain.c
@@ -100,7 +100,6 @@ static int print_object(const struct object_id *oid, const char *path)
 	}
 	ctx.page.filename = path;
 	ctx.page.size = size;
-	ctx.page.etag = oid_to_hex(oid);
 	cgit_print_http_headers();
 	html_raw(buf, size);
 	free(mimetype);
@@ -125,7 +124,6 @@ static void print_dir(const struct object_id *oid, const char *base,
 
 	fullpath = build_path(base, baselen, path);
 	leading_slash = (fullpath[0] == '/' ? "" : "/");
-	ctx.page.etag = oid_to_hex(oid);
 	cgit_print_http_headers();
 	// The listing is a full document of its own, so it carries the same
 	// doctype and charset as the layout pages or the browser would parse
diff --git a/source/ui-shared.c b/source/ui-shared.c
index 12df3b2..74c1c48 100644
--- a/source/ui-shared.c
+++ b/source/ui-shared.c
@@ -315,20 +315,6 @@ static void print_rel_date(time_t t, int tz, double count, const char *class,
 	htmlf("'>%.0f %s</time>", count, suffix);
 }
 
-static char *http_date(time_t t)
-{
-	static char day[][4] =
-		{"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};
-	static char month[][4] =
-		{"Jan", "Feb", "Mar", "Apr", "May", "Jun",
-		 "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"};
-	struct tm tm;
-	gmtime_r(&t, &tm);
-	return cgit_fmt("%s, %02d %s %04d %02d:%02d:%02d GMT", day[tm.tm_wday],
-		   tm.tm_mday, month[tm.tm_mon], 1900 + tm.tm_year,
-		   tm.tm_hour, tm.tm_min, tm.tm_sec);
-}
-
 static void print_rel_vcs_link(const char *url)
 {
 	html("<link rel='vcs-git' href='");
@@ -1119,10 +1105,6 @@ void cgit_print_http_headers(void)
 	}
 	if (!ctx.env.authenticated)
 		html("Cache-Control: no-cache, no-store\n");
-	htmlf("Last-Modified: %s\n", http_date(ctx.page.modified));
-	htmlf("Expires: %s\n", http_date(ctx.page.expires));
-	if (ctx.page.etag)
-		htmlf("ETag: \"%s\"\n", ctx.page.etag);
 	html("\n");
 	// Some pages follow the headers with output written by git itself, not
 	// through html_raw, so the buffer is emptied to keep the headers first.
@@ -1244,7 +1226,6 @@ void cgit_print_error_page(int code, const char *msg, const char *fmt, ...)
 void cgit_vprint_error_page(int code, const char *msg, const char *fmt,
 			    va_list ap)
 {
-	ctx.page.expires = ctx.cfg.cache_dynamic_ttl;
 	ctx.page.status = code;
 	ctx.page.statusmsg = msg;
 	cgit_print_layout_start();
diff --git a/source/ui-snapshot.c b/source/ui-snapshot.c
index 0bab8ea..3a14e16 100644
--- a/source/ui-snapshot.c
+++ b/source/ui-snapshot.c
@@ -206,7 +206,6 @@ static int send_snapshot(const struct cgit_snapshot_format *format,
 				"Not a commit reference: %s", hex);
 		return 1;
 	}
-	ctx.page.etag = oid_to_hex(&oid);
 	ctx.page.mimetype = xstrdup(format->mimetype);
 	ctx.page.filename = xstrdup(filename);
 	cgit_print_http_headers();
@@ -241,7 +240,6 @@ static int send_sig(const struct cgit_snapshot_format *format,
 	// act on.
 	html("X-Content-Type-Options: nosniff\n");
 	html("Content-Security-Policy: default-src 'none'\n");
-	ctx.page.etag = oid_to_hex(note);
 	ctx.page.mimetype = xstrdup("application/pgp-signature");
 	ctx.page.filename = xstrdup(sig_filename);
 	cgit_print_http_headers();
diff --git a/tests/t0010-validate-html.sh b/tests/t0010-validate-html.sh
index 308cef7..842437c 100755
--- a/tests/t0010-validate-html.sh
+++ b/tests/t0010-validate-html.sh
@@ -18,7 +18,7 @@ test_url()
 	cgit_url "$1" >tidy-$test_count.tmp || return
 	# Tidy reads what it is given as a whole document, so the response
 	# headers are dropped before it sees the page.
-	sed -e "1,4d" tidy-$test_count.tmp >tidy-$test_count || return
+	strip_headers <tidy-$test_count.tmp >tidy-$test_count || return
 	"$tidy" $tidy_options tidy-$test_count
 	status=$?
 
diff --git a/tests/t0110-rawdiff.sh b/tests/t0110-rawdiff.sh
index 23d37a5..c546993 100755
--- a/tests/t0110-rawdiff.sh
+++ b/tests/t0110-rawdiff.sh
@@ -18,7 +18,7 @@ test_expect_success 'generate foo/rawdiff' '
 # before the two are compared.
 test_expect_success 'compare with output of git-diff(1)' '
 	git --git-dir="$PWD/repos/foo/.git" diff HEAD^.. >tmp2 &&
-	sed "1,4d" tmp >tmp_ &&
+	strip_headers <tmp >tmp_ &&
 	cmp tmp_ tmp2
 '
 
@@ -32,7 +32,7 @@ test_expect_success 'generate diff for initial commit' '
 
 test_expect_success 'compare with output of git-diff-tree(1)' '
 	git --git-dir="$PWD/repos/foo/.git" diff-tree -p --no-commit-id --root "$root" >tmp2 &&
-	sed "1,4d" tmp >tmp_ &&
+	strip_headers <tmp >tmp_ &&
 	cmp tmp_ tmp2
 '
 
@@ -44,7 +44,7 @@ test_expect_success 'generate diff for multiple commits' '
 
 test_expect_success 'compare with output of git-diff(1)' '
 	git --git-dir="$PWD/repos/foo/.git" diff HEAD~3..HEAD >tmp2 &&
-	sed "1,4d" tmp >tmp_ &&
+	strip_headers <tmp >tmp_ &&
 	cmp tmp_ tmp2
 '