From e07747d8d2cee6639b14ba8021d453a6b8dd9edc Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Mon, 20 Aug 2018 15:55:28 -0400 Subject: [PATCH] playground: prevent caching memory-related runtime errors The existing implementation only prevents code caching if the code has certain compile-time errors. Code with deterministic runtime errors, such as out of memory errors, are being cached. This causes errors to be returned in the Go Tour if a different user's run caused an error. The cache had to be manually cleared in order to clean out the bad run. This change prevents code that threw an "out of memory" or a "cannot allocate memory" error from being cached. Fixes golang/go#26926 Change-Id: Ib281f3076bc674e7c2f08bf9b5c4be36da22d28e Reviewed-on: https://go-review.googlesource.com/130035 Reviewed-by: Andrew Bonventre --- sandbox.go | 15 +++++++++++++++ server_test.go | 13 +++++++++++++ 2 files changed, 28 insertions(+) diff --git a/sandbox.go b/sandbox.go index 36d2f28f..f63c360a 100644 --- a/sandbox.go +++ b/sandbox.go @@ -41,6 +41,9 @@ const ( progName = "prog.go" ) +// Runtime error strings that will not be cached if found in an Event message. +var nonCachingRuntimeErrors = []string{"out of memory", "cannot allocate memory"} + type request struct { Body string } @@ -87,6 +90,18 @@ func (s *server) commandHandler(cachePrefix string, cmdFunc func(*request) (*res http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } + for _, el := range resp.Events { + if el.Kind != "stderr" { + continue + } + for _, e := range nonCachingRuntimeErrors { + if strings.Contains(el.Message, e) { + s.log.Errorf("cmdFunc runtime error: %q", el.Message) + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + return + } + } + } if err := s.cache.Set(key, resp); err != nil { s.log.Errorf("cache.Set(%q, resp): %v", key, err) } diff --git a/server_test.go b/server_test.go index 86217424..63141930 100644 --- a/server_test.go +++ b/server_test.go @@ -166,6 +166,15 @@ func TestCommandHandler(t *testing.T) { if r.Body == "error" { return &response{Errors: "errors"}, nil } + if r.Body == "oom-error" { + // To throw an oom in a local playground instance, increase the server timeout + // to 20 seconds (within sandbox.go), spin up the Docker instance and run + // this code: https://play.golang.org/p/aaCv86m0P14. + return &response{Events: []Event{{"out of memory", "stderr", 0}}}, nil + } + if r.Body == "allocate-memory-error" { + return &response{Events: []Event{{"cannot allocate memory", "stderr", 0}}}, nil + } resp := &response{Events: []Event{{r.Body, "stdout", 0}}} return resp, nil }) @@ -191,6 +200,10 @@ func TestCommandHandler(t *testing.T) { []byte(`{"Errors":"errors","Events":null} `), }, + {"Out of memory error in response body event message", http.MethodPost, http.StatusInternalServerError, + []byte(`{"Body":"oom-error"}`), nil}, + {"Cannot allocate memory error in response body event message", http.MethodPost, http.StatusInternalServerError, + []byte(`{"Body":"allocate-memory-error"}`), nil}, } for _, tc := range testCases {