net/http: add If-None-Match and If-Range support to ServeContent

Also, clear Content-Type and Content-Length on Not Modified
responses before server.go strips them and spams the logs with
warnings.

R=rsc
CC=golang-dev
https://golang.org/cl/6503090

src/pkg/net/http/fs.go

@@ -100,6 +100,9 @@ func dirList(w ResponseWriter, f File) {
 // The content's Seek method must work: ServeContent uses
 // a seek to the end of the content to determine its size.
 //
+// If the caller has set w's ETag header, ServeContent uses it to
+// handle requests using If-Range and If-None-Match.
+//
 // Note that *os.File implements the io.ReadSeeker interface.
 func ServeContent(w ResponseWriter, req *Request, name string, modtime time.Time, content io.ReadSeeker) {
 	size, err := content.Seek(0, os.SEEK_END)
@@ -122,6 +125,10 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time,
 	if checkLastModified(w, r, modtime) {
 		return
 	}
+	rangeReq, done := checkETag(w, r)
+	if done {
+		return
+	}
 
 	code := StatusOK
 
@@ -148,7 +155,7 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time,
 	sendSize := size
 	var sendContent io.Reader = content
 	if size >= 0 {
-		ranges, err := parseRange(r.Header.Get("Range"), size)
+		ranges, err := parseRange(rangeReq, size)
 		if err != nil {
 			Error(w, err.Error(), StatusRequestedRangeNotSatisfiable)
 			return
@@ -240,6 +247,9 @@ func checkLastModified(w ResponseWriter, r *Request, modtime time.Time) bool {
 	// The Date-Modified header truncates sub-second precision, so
 	// use mtime < t+1s instead of mtime <= t to check for unmodified.
 	if t, err := time.Parse(TimeFormat, r.Header.Get("If-Modified-Since")); err == nil && modtime.Before(t.Add(1*time.Second)) {
+		h := w.Header()
+		delete(h, "Content-Type")
+		delete(h, "Content-Length")
 		w.WriteHeader(StatusNotModified)
 		return true
 	}
@@ -247,6 +257,58 @@ func checkLastModified(w ResponseWriter, r *Request, modtime time.Time) bool {
 	return false
 }
 
+// checkETag implements If-None-Match and If-Range checks.
+// The ETag must have been previously set in the ResponseWriter's headers.
+//
+// The return value is the effective request "Range" header to use and
+// whether this request is now considered done.
+func checkETag(w ResponseWriter, r *Request) (rangeReq string, done bool) {
+	etag := w.Header().get("Etag")
+	rangeReq = r.Header.get("Range")
+
+	// Invalidate the range request if the entity doesn't match the one
+	// the client was expecting.
+	// "If-Range: version" means "ignore the Range: header unless version matches the
+	// current file."
+	// We only support ETag versions.
+	// The caller must have set the ETag on the response already.
+	if ir := r.Header.get("If-Range"); ir != "" && ir != etag {
+		// TODO(bradfitz): handle If-Range requests with Last-Modified
+		// times instead of ETags? I'd rather not, at least for
+		// now. That seems like a bug/compromise in the RFC 2616, and
+		// I've never heard of anybody caring about that (yet).
+		rangeReq = ""
+	}
+
+	if inm := r.Header.get("If-None-Match"); inm != "" {
+		// Must know ETag.
+		if etag == "" {
+			return rangeReq, false
+		}
+
+		// TODO(bradfitz): non-GET/HEAD requests require more work:
+		// sending a different status code on matches, and
+		// also can't use weak cache validators (those with a "W/
+		// prefix).  But most users of ServeContent will be using
+		// it on GET or HEAD, so only support those for now.
+		if r.Method != "GET" && r.Method != "HEAD" {
+			return rangeReq, false
+		}
+
+		// TODO(bradfitz): deal with comma-separated or multiple-valued
+		// list of If-None-match values.  For now just handle the common
+		// case of a single item.
+		if inm == etag || inm == "*" {
+			h := w.Header()
+			delete(h, "Content-Type")
+			delete(h, "Content-Length")
+			w.WriteHeader(StatusNotModified)
+			return "", true
+		}
+	}
+	return rangeReq, false
+}
+
 // name is '/'-separated, not filepath.Separator.
 func serveFile(w ResponseWriter, r *Request, fs FileSystem, name string, redirect bool) {
 	const indexPage = "/index.html"

src/pkg/net/http/fs_test.go

@@ -527,51 +527,140 @@ func TestDirectoryIfNotModified(t *testing.T) {
 	res.Body.Close()
 }
 
-func TestServeContent(t *testing.T) {
+func mustStat(t *testing.T, fileName string) os.FileInfo {
-	type req struct {
+	fi, err := os.Stat(fileName)
-		name    string
+	if err != nil {
-		modtime time.Time
+		t.Fatal(err)
-		content io.ReadSeeker
 	}
-	ch := make(chan req, 1)
+	return fi
+}
+
+func TestServeContent(t *testing.T) {
+	type serveParam struct {
+		name        string
+		modtime     time.Time
+		content     io.ReadSeeker
+		contentType string
+		etag        string
+	}
+	servec := make(chan serveParam, 1)
 	ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
-		p := <-ch
+		p := <-servec
+		if p.etag != "" {
+			w.Header().Set("ETag", p.etag)
+		}
+		if p.contentType != "" {
+			w.Header().Set("Content-Type", p.contentType)
+		}
 		ServeContent(w, r, p.name, p.modtime, p.content)
 	}))
 	defer ts.Close()
 
-	css, err := os.Open("testdata/style.css")
+	type testCase struct {
-	if err != nil {
+		file             string
-		t.Fatal(err)
+		modtime          time.Time
+		serveETag        string // optional
+		serveContentType string // optional
+		reqHeader        map[string]string
+		wantLastMod      string
+		wantContentType  string
+		wantStatus       int
 	}
-	defer css.Close()
+	htmlModTime := mustStat(t, "testdata/index.html").ModTime()
+	tests := map[string]testCase{
+		"no_last_modified": {
+			file:            "testdata/style.css",
+			wantContentType: "text/css; charset=utf-8",
+			wantStatus:      200,
+		},
+		"with_last_modified": {
+			file:            "testdata/index.html",
+			wantContentType: "text/html; charset=utf-8",
+			modtime:         htmlModTime,
+			wantLastMod:     htmlModTime.UTC().Format(TimeFormat),
+			wantStatus:      200,
+		},
+		"not_modified_modtime": {
+			file:    "testdata/style.css",
+			modtime: htmlModTime,
+			reqHeader: map[string]string{
+				"If-Modified-Since": htmlModTime.UTC().Format(TimeFormat),
+			},
+			wantStatus: 304,
+		},
+		"not_modified_modtime_with_contenttype": {
+			file:             "testdata/style.css",
+			serveContentType: "text/css", // explicit content type
+			modtime:          htmlModTime,
+			reqHeader: map[string]string{
+				"If-Modified-Since": htmlModTime.UTC().Format(TimeFormat),
+			},
+			wantStatus: 304,
+		},
+		"not_modified_etag": {
+			file:      "testdata/style.css",
+			serveETag: `"foo"`,
+			reqHeader: map[string]string{
+				"If-None-Match": `"foo"`,
+			},
+			wantStatus: 304,
+		},
+		"range_good": {
+			file:      "testdata/style.css",
+			serveETag: `"A"`,
+			reqHeader: map[string]string{
+				"Range": "bytes=0-4",
+			},
+			wantStatus:      StatusPartialContent,
+			wantContentType: "text/css; charset=utf-8",
+		},
+		// An If-Range resource for entity "A", but entity "B" is now current.
+		// The Range request should be ignored.
+		"range_no_match": {
+			file:      "testdata/style.css",
+			serveETag: `"A"`,
+			reqHeader: map[string]string{
+				"Range":    "bytes=0-4",
+				"If-Range": `"B"`,
+			},
+			wantStatus:      200,
+			wantContentType: "text/css; charset=utf-8",
+		},
+	}
+	for testName, tt := range tests {
+		f, err := os.Open(tt.file)
+		if err != nil {
+			t.Fatalf("test %q: %v", testName, err)
+		}
+		defer f.Close()
 
-	ch <- req{"style.css", time.Time{}, css}
+		servec <- serveParam{
-	res, err := Get(ts.URL)
+			name:        filepath.Base(tt.file),
-	if err != nil {
+			content:     f,
-		t.Fatal(err)
+			modtime:     tt.modtime,
-	}
+			etag:        tt.serveETag,
-	if g, e := res.Header.Get("Content-Type"), "text/css; charset=utf-8"; g != e {
+			contentType: tt.serveContentType,
-		t.Errorf("style.css: content type = %q, want %q", g, e)
+		}
-	}
+		req, err := NewRequest("GET", ts.URL, nil)
-	if g := res.Header.Get("Last-Modified"); g != "" {
+		if err != nil {
-		t.Errorf("want empty Last-Modified; got %q", g)
+			t.Fatal(err)
-	}
+		}
-
+		for k, v := range tt.reqHeader {
-	fi, err := css.Stat()
+			req.Header.Set(k, v)
-	if err != nil {
+		}
-		t.Fatal(err)
+		res, err := DefaultClient.Do(req)
-	}
+		if err != nil {
-	ch <- req{"style.html", fi.ModTime(), css}
+			t.Fatal(err)
-	res, err = Get(ts.URL)
+		}
-	if err != nil {
+		if res.StatusCode != tt.wantStatus {
-		t.Fatal(err)
+			t.Errorf("test %q: status = %d; want %d", testName, res.StatusCode, tt.wantStatus)
-	}
+		}
-	if g, e := res.Header.Get("Content-Type"), "text/html; charset=utf-8"; g != e {
+		if g, e := res.Header.Get("Content-Type"), tt.wantContentType; g != e {
-		t.Errorf("style.html: content type = %q, want %q", g, e)
+			t.Errorf("test %q: content-type = %q, want %q", testName, g, e)
-	}
+		}
-	if g := res.Header.Get("Last-Modified"); g == "" {
+		if g, e := res.Header.Get("Last-Modified"), tt.wantLastMod; g != e {
-		t.Errorf("want non-empty last-modified")
+			t.Errorf("test %q: last-modified = %q, want %q", testName, g, e)
+		}
 	}
 }