diff --git a/godoc/meta.go b/godoc/meta.go index 41ade3948f..260833dbd1 100644 --- a/godoc/meta.go +++ b/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 { diff --git a/godoc/server.go b/godoc/server.go index 17514418c9..8724291c6c 100644 --- a/godoc/server.go +++ b/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 diff --git a/godoc/server_test.go b/godoc/server_test.go index a7ed5142f5..f8621352f0 100644 --- a/godoc/server_test.go +++ b/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": ` + +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) + } + } +}