test-results: Get handler cleanup

go/src/infra/appengine/test-results/frontend/get.go

Index: go/src/infra/appengine/test-results/frontend/get.go
diff --git a/go/src/infra/appengine/test-results/frontend/get.go b/go/src/infra/appengine/test-results/frontend/get.go
index 6e0ed6ba694846c38142978a265c526376abee5a..9e2f4cf506b97b35f6879f0fa992e9f338ac1334 100644
--- a/go/src/infra/appengine/test-results/frontend/get.go
+++ b/go/src/infra/appengine/test-results/frontend/get.go
@@ -7,7 +7,6 @@ import (
 	"infra/appengine/test-results/model"
 	"io"
 	"net/http"
-	"regexp"
 	"time"
 
 	"github.com/luci/gae/service/datastore"
@@ -31,12 +30,8 @@ const (
 	httpNoTZTimeFormat = "Mon, 02 Jan 2006 15:04:05"
 )
 
-// callbackNameRx matches start of strings that look like
-// JavaScript function names. Not a comprehensive solution.
-var callbackNameRx = regexp.MustCompile(`^[A-Za-z0-9_]+$`)

[estaab, 2016/08/16 15:33:23]

What's the consequence of removing this? Any security concerns?

-
-// GetHandler is the HTTP handler for GET /testfile requests.
-func GetHandler(ctx *router.Context) {
+// getHandler is the HTTP handler for GET /testfile requests.
+func getHandler(ctx *router.Context) {
 	c, w, r := ctx.Context, ctx.Writer, ctx.Request
 	if err := r.ParseForm(); err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
@@ -75,11 +70,12 @@ func respondTestFileData(ctx *router.Context, params URLParams) {
 
 	tf := model.TestFile{ID: key.IntID()}
 
-	if err := datastore.Get(c).Get(&tf); err == datastore.ErrNoSuchEntity {
-		http.Error(w, err.Error(), http.StatusNotFound)
-		logging.Errorf(c, "TestFile with ID %v not found: %v", key.IntID(), err)
-		return
-	} else if err != nil {
+	if err := datastore.Get(c).Get(&tf); err != nil {
+		if err == datastore.ErrNoSuchEntity {
+			http.Error(w, err.Error(), http.StatusNotFound)
+			logging.Errorf(c, "TestFile with ID %v not found: %v", key.IntID(), err)
+			return
+		}
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		logging.Errorf(c, "failed to get TestFile with ID %v: %v", key.IntID(), err)
 		return
@@ -115,15 +111,6 @@ func respondTestFileList(ctx *router.Context, params URLParams) {
 		return
 	}
 
-	args := templates.Args{
-		"Master":      params.Master,
-		"Builder":     params.Builder,
-		"TestType":    params.TestType,
-		"BuildNumber": params.BuildNumber,
-		"Name":        params.Name,
-		"Files":       testFiles,
-	}
-
 	if params.Callback != "" {
 		b, err := keysJSON(c, testFiles)
 		if err != nil {
@@ -135,7 +122,14 @@ func respondTestFileList(ctx *router.Context, params URLParams) {
 		return
 	}
 
-	templates.MustRender(c, w, "pages/showfilelist.html", args)
+	templates.MustRender(c, w, "pages/showfilelist.html", templates.Args{
+		"Master":      params.Master,
+		"Builder":     params.Builder,
+		"TestType":    params.TestType,
+		"BuildNumber": params.BuildNumber,
+		"Name":        params.Name,
+		"Files":       testFiles,
+	})
 }
 
 func keysJSON(c context.Context, tfiles []*model.TestFile) ([]byte, error) {
@@ -221,9 +215,10 @@ func respondTestFileDefault(ctx *router.Context, params URLParams) {
 			logging.Errorf(c, "failed to unmarshal test results JSON: %+v: %v", data, err)
 			return
 		}
-		aggr.Tests.ToTestList()
+
+		tl := aggr.TestList()
 		buf := &bytes.Buffer{}
-		if err := json.NewEncoder(buf).Encode(aggr.Tests); err != nil {
+		if err := json.NewEncoder(buf).Encode(&tl); err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 			logging.Errorf(c, "failed to marshal test list JSON: %+v, %v", aggr.Tests, err)
 			return
@@ -258,11 +253,11 @@ func getFirstTestFile(c context.Context, q *datastore.Query) (*model.TestFile, e
 	return tfs[0], nil
 }
 
-// respondJSON writes the supplied JSON data to w. If the supplied callback string matches
-// callbackNameRx, data is wrapped in a JSONP-style function with the supplied callback
-// string as the function name.
+// respondJSON writes the supplied JSON data to w. If the supplied
+// callback string is not empty, data is wrapped in a JSONP-style
+// function with the supplied callback  string as the function name.
 func respondJSON(c context.Context, w http.ResponseWriter, data io.Reader, lastMod time.Time, callback string) {
-	if callbackNameRx.MatchString(callback) {
+	if callback != "" {
 		data = wrapCallback(data, callback)
 	}
 	w.Header().Set("Last-Modified", lastMod.Format(httpNoTZTimeFormat)+" GMT")