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>
diff --git a/godoc/meta.go b/godoc/meta.go
index 41ade39..260833d 100644
--- a/godoc/meta.go
+++ b/godoc/meta.go
@@ -7,7 +7,9 @@
import (
"bytes"
"encoding/json"
+ "errors"
"log"
+ "os"
pathpkg "path"
"strings"
"time"
@@ -64,7 +66,11 @@
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 1751441..8724291 100644
--- a/godoc/server.go
+++ b/godoc/server.go
@@ -754,9 +754,15 @@
}
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 @@
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 a7ed514..f862135 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 @@
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)
+ }
+ }
+}
