godoc: fix panic in Presentation.ServeFile

The redirect to drop index.html must be done using r.URL.Path,
not relpath, because those might differ. Cutting len("index.html")
bytes off a string that doesn't end in index.html is incorrect.

While we're here, silence an annoying log print during go test.

For golang/go#40665.

Change-Id: I36553b041f53eab9c42da6b77184e90800a97e92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/251080
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

godoc/meta.go

@@ -7,7 +7,9 @@ package godoc
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"log"
+	"os"
 	pathpkg "path"
 	"strings"
 	"time"
@@ -64,7 +66,11 @@ func (c *Corpus) updateMetadata() {
 	scan = func(dir string) {
 		fis, err := c.fs.ReadDir(dir)
 		if err != nil {
-			log.Println("updateMetadata:", err)
+			if dir == "/doc" && errors.Is(err, os.ErrNotExist) {
+				// Be quiet during tests that don't have a /doc tree.
+				return
+			}
+			log.Printf("updateMetadata %s: %v", dir, err)
 			return
 		}
 		for _, fi := range fis {

godoc/server.go

@@ -754,9 +754,15 @@ func (p *Presentation) ServeFile(w http.ResponseWriter, r *http.Request) {
 }
 
 func (p *Presentation) serveFile(w http.ResponseWriter, r *http.Request) {
-	relpath := r.URL.Path
+	if strings.HasSuffix(r.URL.Path, "/index.html") {
+		// We'll show index.html for the directory.
+		// Use the dir/ version as canonical instead of dir/index.html.
+		http.Redirect(w, r, r.URL.Path[0:len(r.URL.Path)-len("index.html")], http.StatusMovedPermanently)
+		return
+	}
 
 	// Check to see if we need to redirect or serve another file.
+	relpath := r.URL.Path
 	if m := p.Corpus.MetadataFor(relpath); m != nil {
 		if m.Path != relpath {
 			// Redirect to canonical path.
@@ -772,12 +778,6 @@ func (p *Presentation) serveFile(w http.ResponseWriter, r *http.Request) {
 
 	switch pathpkg.Ext(relpath) {
 	case ".html":
-		if strings.HasSuffix(relpath, "/index.html") {
-			// We'll show index.html for the directory.
-			// Use the dir/ version as canonical instead of dir/index.html.
-			http.Redirect(w, r, r.URL.Path[0:len(r.URL.Path)-len("index.html")], http.StatusMovedPermanently)
-			return
-		}
 		p.ServeHTMLDoc(w, r, abspath, relpath)
 		return
 

godoc/server_test.go

@@ -5,7 +5,12 @@
 package godoc
 
 import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
 	"testing"
+	"text/template"
 
 	"golang.org/x/tools/godoc/vfs/mapfs"
 )
@@ -67,3 +72,41 @@ func F()
 		t.Errorf("pInfo.PDoc.Funcs[0].Doc = %q; want %q", got, want)
 	}
 }
+
+func TestRedirectAndMetadata(t *testing.T) {
+	c := NewCorpus(mapfs.New(map[string]string{
+		"doc/y/index.html": "Hello, y.",
+		"doc/x/index.html": `<!--{
+		"Path": "/doc/x/"
+}-->
+
+Hello, x.
+`}))
+	c.updateMetadata()
+	p := &Presentation{
+		Corpus:    c,
+		GodocHTML: template.Must(template.New("").Parse(`{{printf "%s" .Body}}`)),
+	}
+	r := &http.Request{URL: &url.URL{}}
+
+	// Test that redirect is sent back correctly.
+	// Used to panic. See golang.org/issue/40665.
+	for _, elem := range []string{"x", "y"} {
+		dir := "/doc/" + elem + "/"
+		r.URL.Path = dir + "index.html"
+		rw := httptest.NewRecorder()
+		p.ServeFile(rw, r)
+		loc := rw.Result().Header.Get("Location")
+		if rw.Code != 301 || loc != dir {
+			t.Errorf("GET %s: expected 301 -> %q, got %d -> %q", r.URL.Path, dir, rw.Code, loc)
+		}
+
+		r.URL.Path = dir
+		rw = httptest.NewRecorder()
+		p.ServeFile(rw, r)
+		if rw.Code != 200 || !strings.Contains(rw.Body.String(), "Hello, "+elem) {
+			t.Fatalf("GET %s: expected 200 w/ Hello, %s: got %d w/ body:\n%s",
+				r.URL.Path, elem, rw.Code, rw.Body)
+		}
+	}
+}