From e715e52d3192ffe661ac708996d63effba6e6f25 Mon Sep 17 00:00:00 2001 From: Denis Sedchenko Date: Mon, 15 May 2023 17:53:20 +0200 Subject: [PATCH 1/3] feat: properly filter content length (#244) --- internal/langserver/errors.go | 9 +++++ internal/langserver/middleware.go | 10 ------ internal/langserver/request.go | 11 ------- internal/langserver/server.go | 55 +++++++++++++++++++++++-------- pkg/goplay/client.go | 14 +++----- pkg/goplay/methods.go | 8 ----- pkg/goplay/methods_test.go | 19 ----------- 7 files changed, 54 insertions(+), 72 deletions(-) diff --git a/internal/langserver/errors.go b/internal/langserver/errors.go index 93c7f5a8..33d14885 100644 --- a/internal/langserver/errors.go +++ b/internal/langserver/errors.go @@ -3,6 +3,15 @@ package langserver import ( "fmt" "net/http" + + "github.com/x1unix/go-playground/pkg/goplay" +) + +// ErrSnippetTooLarge is snippet max size limit error +var ErrSnippetTooLarge = Errorf( + http.StatusRequestEntityTooLarge, + "code snippet too large (max %d bytes)", + goplay.MaxSnippetSize, ) // HTTPError is HTTP response error diff --git a/internal/langserver/middleware.go b/internal/langserver/middleware.go index 250a08fe..3603a99d 100644 --- a/internal/langserver/middleware.go +++ b/internal/langserver/middleware.go @@ -2,7 +2,6 @@ package langserver import ( "errors" - "github.com/x1unix/go-playground/pkg/goplay" "net/http" "syscall" ) @@ -29,15 +28,6 @@ func WrapHandler(h HandlerFunc, guards ...GuardFn) http.HandlerFunc { } } -// ValidateContentLength validates Go code snippet size -func ValidateContentLength(r *http.Request) error { - if err := goplay.ValidateContentLength(int(r.ContentLength)); err != nil { - return NewHTTPError(http.StatusRequestEntityTooLarge, err) - } - - return nil -} - func handleError(err error, w http.ResponseWriter) { if err == nil { return diff --git a/internal/langserver/request.go b/internal/langserver/request.go index c004a361..76e11cda 100644 --- a/internal/langserver/request.go +++ b/internal/langserver/request.go @@ -3,7 +3,6 @@ package langserver import ( "encoding/json" "go.uber.org/zap" - "io" "net/http" "strconv" ) @@ -60,13 +59,3 @@ func shouldFormatCode(r *http.Request) (bool, error) { return boolVal, nil } - -func getPayloadFromRequest(r *http.Request) ([]byte, error) { - src, err := io.ReadAll(r.Body) - if err != nil { - return nil, Errorf(http.StatusBadGateway, "failed to read request: %s", err) - } - - r.Body.Close() - return src, nil -} diff --git a/internal/langserver/server.go b/internal/langserver/server.go index 74096514..c8e853c9 100644 --- a/internal/langserver/server.go +++ b/internal/langserver/server.go @@ -1,6 +1,7 @@ package langserver import ( + "bytes" "context" "errors" "fmt" @@ -70,13 +71,13 @@ func (s *Service) Mount(r *mux.Router) { r.Path("/suggest"). HandlerFunc(WrapHandler(s.HandleGetSuggestion)) r.Path("/run").Methods(http.MethodPost). - HandlerFunc(WrapHandler(s.HandleRunCode, ValidateContentLength)) + HandlerFunc(WrapHandler(s.HandleRunCode)) r.Path("/compile").Methods(http.MethodPost). - HandlerFunc(WrapHandler(s.HandleCompile, ValidateContentLength)) + HandlerFunc(WrapHandler(s.HandleCompile)) r.Path("/format").Methods(http.MethodPost). - HandlerFunc(WrapHandler(s.HandleFormatCode, ValidateContentLength)) + HandlerFunc(WrapHandler(s.HandleFormatCode)) r.Path("/share").Methods(http.MethodPost). - HandlerFunc(WrapHandler(s.HandleShare, ValidateContentLength)) + HandlerFunc(WrapHandler(s.HandleShare)) r.Path("/snippet/{id}").Methods(http.MethodGet). HandlerFunc(WrapHandler(s.HandleGetSnippet)) r.Path("/backends/info").Methods(http.MethodGet). @@ -159,7 +160,7 @@ func (s *Service) HandleGetSuggestion(w http.ResponseWriter, r *http.Request) er // HandleFormatCode handles goimports action func (s *Service) HandleFormatCode(w http.ResponseWriter, r *http.Request) error { - src, err := getPayloadFromRequest(r) + src, err := s.getPayloadFromRequest(r) if err != nil { return err } @@ -186,8 +187,8 @@ func (s *Service) HandleShare(w http.ResponseWriter, r *http.Request) error { shareID, err := s.client.Share(r.Context(), r.Body) defer r.Body.Close() if err != nil { - if errors.Is(err, goplay.ErrSnippetTooLarge) { - return NewHTTPError(http.StatusRequestEntityTooLarge, err) + if isContentLengthError(err) { + return ErrSnippetTooLarge } s.log.Error("failed to share code: ", err) @@ -225,7 +226,7 @@ func (s *Service) HandleGetSnippet(w http.ResponseWriter, r *http.Request) error // HandleRunCode handles code run func (s *Service) HandleRunCode(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() - src, err := getPayloadFromRequest(r) + src, err := s.getPayloadFromRequest(r) if err != nil { return err } @@ -304,9 +305,8 @@ func (s *Service) HandleArtifactRequest(w http.ResponseWriter, r *http.Request) w.Header().Set("Content-Length", contentLength) w.Header().Set(rawContentLengthHeader, contentLength) - n, err := io.Copy(w, data) defer data.Close() - if err != nil { + if _, err := io.Copy(w, data); err != nil { s.log.Errorw("failed to send artifact", "artifactID", artifactId, "err", err, @@ -314,7 +314,6 @@ func (s *Service) HandleArtifactRequest(w http.ResponseWriter, r *http.Request) return err } - w.Header().Set("Content-Length", strconv.FormatInt(n, 10)) return nil } @@ -329,7 +328,7 @@ func (s *Service) HandleCompile(w http.ResponseWriter, r *http.Request) error { return NewHTTPError(http.StatusTooManyRequests, err) } - src, err := getPayloadFromRequest(r) + src, err := s.getPayloadFromRequest(r) if err != nil { return err } @@ -391,8 +390,8 @@ func backendFromRequest(r *http.Request) (goplay.Backend, error) { func (s *Service) goImportsCode(ctx context.Context, src []byte, backend goplay.Backend) ([]byte, bool, error) { resp, err := s.client.GoImports(ctx, src, backend) if err != nil { - if errors.Is(err, goplay.ErrSnippetTooLarge) { - return nil, false, NewHTTPError(http.StatusRequestEntityTooLarge, err) + if isContentLengthError(err) { + return nil, false, ErrSnippetTooLarge } s.log.Error(err) @@ -406,3 +405,31 @@ func (s *Service) goImportsCode(ctx context.Context, src []byte, backend goplay. changed := resp.Body != string(src) return []byte(resp.Body), changed, nil } + +func (s *Service) getPayloadFromRequest(r *http.Request) ([]byte, error) { + // see: https://github.com/golang/playground/blob/master/share.go#L69 + var buff bytes.Buffer + buff.Grow(goplay.MaxSnippetSize) + + defer r.Body.Close() + _, err := io.Copy(&buff, io.LimitReader(r.Body, goplay.MaxSnippetSize+1)) + if err != nil { + return nil, Errorf(http.StatusBadGateway, "failed to read request: %w", err) + } + + if buff.Len() > goplay.MaxSnippetSize { + return nil, ErrSnippetTooLarge + } + + return buff.Bytes(), nil +} + +func isContentLengthError(err error) bool { + if httpErr, ok := goplay.IsHTTPError(err); ok { + if httpErr.StatusCode == http.StatusRequestEntityTooLarge { + return true + } + } + + return false +} diff --git a/pkg/goplay/client.go b/pkg/goplay/client.go index e6f933e2..9e7e9bc1 100644 --- a/pkg/goplay/client.go +++ b/pkg/goplay/client.go @@ -17,14 +17,11 @@ const ( DefaultUserAgent = "goplay.tools/1.0 (http://goplay.tools/)" DefaultPlaygroundURL = "https://go.dev/_" - // maxSnippetSize value taken from + // MaxSnippetSize value taken from // https://github.com/golang/playground/blob/master/app/goplay/share.go - maxSnippetSize = 64 * 1024 + MaxSnippetSize = 64 * 1024 ) -// ErrSnippetTooLarge is snippet max size limit error -var ErrSnippetTooLarge = fmt.Errorf("code snippet too large (max %d bytes)", maxSnippetSize) - // Client is Go Playground API client type Client struct { client http.Client @@ -89,14 +86,11 @@ func (c *Client) doRequest(ctx context.Context, method, url, contentType string, return nil, NewHTTPError(response) } - bodyBytes := &bytes.Buffer{} - _, err = io.Copy(bodyBytes, io.LimitReader(response.Body, maxSnippetSize+1)) + bodyBytes := bytes.Buffer{} + _, err = io.Copy(&bodyBytes, response.Body) if err != nil { return nil, err } - if err = ValidateContentLength(bodyBytes.Len()); err != nil { - return nil, err - } return bodyBytes.Bytes(), nil } diff --git a/pkg/goplay/methods.go b/pkg/goplay/methods.go index 79562369..2259b7dd 100644 --- a/pkg/goplay/methods.go +++ b/pkg/goplay/methods.go @@ -8,14 +8,6 @@ import ( "net/url" ) -// ValidateContentLength validates snippet size -func ValidateContentLength(itemLen int) error { - if itemLen > maxSnippetSize { - return ErrSnippetTooLarge - } - return nil -} - // GetSnippet returns snippet from Go playground func (c *Client) GetSnippet(ctx context.Context, snippetID string) (*Snippet, error) { fileName := snippetID + ".go" diff --git a/pkg/goplay/methods_test.go b/pkg/goplay/methods_test.go index 093b8664..754f3d6b 100644 --- a/pkg/goplay/methods_test.go +++ b/pkg/goplay/methods_test.go @@ -7,7 +7,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "strconv" "testing" "time" @@ -16,24 +15,6 @@ import ( "github.com/x1unix/go-playground/pkg/testutil" ) -func TestValidateContentLength(t *testing.T) { - cases := map[int]bool{ - maxSnippetSize: false, - maxSnippetSize + 10: true, - 10: false, - } - for i, c := range cases { - t.Run(strconv.Itoa(i), func(t *testing.T) { - err := ValidateContentLength(i) - if !c { - require.NoError(t, err) - return - } - require.Error(t, err) - }) - } -} - func TestClient_Compile(t *testing.T) { cases := map[string]struct { expect *CompileResponse From 6e02e7cda5c24482b25af3f41f405cffe4ee55ce Mon Sep 17 00:00:00 2001 From: Denis Sedchenko Date: Mon, 15 May 2023 18:49:43 +0200 Subject: [PATCH 2/3] fix: properly handle Go execution timeout (#246) --- web/src/store/dispatchers/build.ts | 60 +++++++++++++++++++++++++----- web/src/utils/duration.ts | 5 +++ 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/web/src/store/dispatchers/build.ts b/web/src/store/dispatchers/build.ts index 28f732cd..77bbf1a3 100644 --- a/web/src/store/dispatchers/build.ts +++ b/web/src/store/dispatchers/build.ts @@ -1,7 +1,7 @@ import {TargetType} from '~/services/config'; import {getWorkerInstance} from "~/services/gorepl"; import {getImportObject, goRun} from '~/services/go'; -import {setTimeoutNanos} from "~/utils/duration"; +import {setTimeoutNanos, SECOND} from "~/utils/duration"; import client, { EvalEvent, EvalEventKind, @@ -29,6 +29,29 @@ import {wrapResponseWithProgress} from "~/utils/http"; const WASM_APP_DOWNLOAD_NOTIFICATION = 'WASM_APP_DOWNLOAD_NOTIFICATION'; const WASM_APP_EXIT_ERROR = 'WASM_APP_EXIT_ERROR'; +/** + * Go program execution timeout in nanoseconds + */ +const runTimeoutNs = 5 * SECOND; + +const lastElem = (items: T[]): T|undefined => ( + items?.slice(-1)?.[0] +); + +const hasProgramTimeoutError = (events: EvalEvent[]) => { + if (!events.length) { + return false; + } + + const { Message, Kind } = events[0]; + if (Kind === 'stderr' && Message.trim() === 'timeout running program') { + const lastEvent = lastElem(events); + return lastEvent!.Delay >= runTimeoutNs; + } + + return false; +} + const dispatchEvalEvents = (dispatch: DispatchFn, events: EvalEvent[]) => { // TODO: support cancellation @@ -39,20 +62,37 @@ const dispatchEvalEvents = (dispatch: DispatchFn, events: EvalEvent[]) => { // Each eval event contains time since previous event. // Convert relative delay into absolute delay since program start. - const eventsWithDelay = events.map((event, i, arr) => ( - i === 0 ? event : ( - { - ...event, - Delay: arr[i - 1].Delay + event.Delay - } - ) - )); + let eventsWithDelay = events + .reduce((accum: EvalEvent[], {Delay: delay, ...item}) => ( + [ + ...accum, + { + ...item, + Delay: (lastElem(accum)?.Delay ?? 0) + delay, + } + ] + ), []); + + // Sometimes Go playground fails to detect execution timeout error and still sends all events. + // This dirty hack attempts to normalize this case. + if (hasProgramTimeoutError(eventsWithDelay)) { + eventsWithDelay = eventsWithDelay + .slice(1) + .filter(({Delay}) => Delay <= runTimeoutNs) + .concat({ + Kind: EvalEventKind.Stderr, + Message: `Go program execution timeout exceeded (max: ${runTimeoutNs / SECOND}s)`, + Delay: runTimeoutNs, + }); + } + + console.log(eventsWithDelay); // Try to guess program end time by checking last message delay. // // This won't work if "time.Sleep()" occurs after final message but the same // approach used in official playground, so should be enough for us. - const programEndTime = eventsWithDelay?.slice(-1)?.[0]?.Delay ?? 0; + let programEndTime = lastElem(eventsWithDelay)?.Delay ?? 0; dispatch(newProgramStartAction()); eventsWithDelay.forEach(event => { diff --git a/web/src/utils/duration.ts b/web/src/utils/duration.ts index a4de620c..15ed6847 100644 --- a/web/src/utils/duration.ts +++ b/web/src/utils/duration.ts @@ -3,6 +3,11 @@ */ export const MSEC_IN_NANOSEC = 1000000; +/** + * Number of nanoseconds in a second. + */ +export const SECOND = 1000 * MSEC_IN_NANOSEC; + /** * Converts nanoseconds to milliseconds * @param ns Delay in anoseconds From 182a2b0f3bf82845ea4db1097d586d55f1a9bd4d Mon Sep 17 00:00:00 2001 From: x1unix Date: Mon, 15 May 2023 18:50:42 +0200 Subject: [PATCH 3/3] fix: fix typo --- web/src/store/dispatchers/build.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/web/src/store/dispatchers/build.ts b/web/src/store/dispatchers/build.ts index 77bbf1a3..fcc24e4d 100644 --- a/web/src/store/dispatchers/build.ts +++ b/web/src/store/dispatchers/build.ts @@ -86,8 +86,6 @@ const dispatchEvalEvents = (dispatch: DispatchFn, events: EvalEvent[]) => { }); } - console.log(eventsWithDelay); - // Try to guess program end time by checking last message delay. // // This won't work if "time.Sleep()" occurs after final message but the same