From 62bbde7936109d18bf1f727974719804dad4c105 Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Thu, 3 Jun 2021 14:54:06 +0300 Subject: - Do not write an error into the responseWriter if this is internal error - Handle SoftJob error Signed-off-by: Valery Piashchynski --- tests/issue659.php | 21 +++++++ tests/plugins/http/configs/.rr-issue659.yaml | 22 +++++++ tests/plugins/http/handler_test.go | 7 ++- tests/plugins/http/http_plugin_test.go | 86 +++++++++++++++++++++++++++- 4 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 tests/issue659.php create mode 100644 tests/plugins/http/configs/.rr-issue659.yaml (limited to 'tests') diff --git a/tests/issue659.php b/tests/issue659.php new file mode 100644 index 00000000..2a0e4f19 --- /dev/null +++ b/tests/issue659.php @@ -0,0 +1,21 @@ +waitRequest()) { + $psr7->getWorker()->error("test_error"); +} diff --git a/tests/plugins/http/configs/.rr-issue659.yaml b/tests/plugins/http/configs/.rr-issue659.yaml new file mode 100644 index 00000000..6e0584a5 --- /dev/null +++ b/tests/plugins/http/configs/.rr-issue659.yaml @@ -0,0 +1,22 @@ +rpc: + listen: tcp://127.0.0.1:6001 + +server: + command: "php ../../issue659.php" + relay: "pipes" + relay_timeout: "20s" + +http: + address: 127.0.0.1:32552 + max_request_size: 1024 + middleware: [ ] + uploads: + forbid: [ ".php", ".exe", ".bat" ] + trusted_subnets: [ "10.0.0.0/8", "127.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "::1/128", "fc00::/7", "fe80::/10" ] + pool: + num_workers: 1 + +logs: + mode: development + level: debug + diff --git a/tests/plugins/http/handler_test.go b/tests/plugins/http/handler_test.go index f6533dc4..1b7f5bac 100644 --- a/tests/plugins/http/handler_test.go +++ b/tests/plugins/http/handler_test.go @@ -1357,7 +1357,7 @@ func TestHandler_Error3(t *testing.T) { }() assert.NoError(t, err) - assert.Equal(t, 500, r.StatusCode) + assert.Equal(t, 400, r.StatusCode) } func TestHandler_ResponseDuration(t *testing.T) { @@ -1519,12 +1519,12 @@ func TestHandler_ErrorDuration(t *testing.T) { }() time.Sleep(time.Millisecond * 10) - goterr := make(chan interface{}) + goterr := make(chan struct{}, 10) h.AddListener(func(event interface{}) { switch tp := event.(type) { case handler.ErrorEvent: if tp.Elapsed() > 0 { - close(goterr) + goterr <- struct{}{} } default: } @@ -1536,6 +1536,7 @@ func TestHandler_ErrorDuration(t *testing.T) { _ = r.Body.Close() }() + <-goterr <-goterr assert.Equal(t, 500, r.StatusCode) diff --git a/tests/plugins/http/http_plugin_test.go b/tests/plugins/http/http_plugin_test.go index 128eec26..f8044c97 100644 --- a/tests/plugins/http/http_plugin_test.go +++ b/tests/plugins/http/http_plugin_test.go @@ -1557,7 +1557,7 @@ func bigEchoHTTP(t *testing.T) { assert.NoError(t, err) b, err := ioutil.ReadAll(r.Body) assert.NoError(t, err) - assert.Equal(t, 500, r.StatusCode) + assert.Equal(t, 400, r.StatusCode) assert.Equal(t, "http_handler_max_size: request body max size is exceeded\n", string(b)) err = r.Body.Close() @@ -2130,6 +2130,90 @@ func staticFilesForbid(t *testing.T) { _ = r.Body.Close() } +func TestHTTPIssue659(t *testing.T) { + cont, err := endure.NewContainer(nil, endure.SetLogLevel(endure.ErrorLevel)) + assert.NoError(t, err) + + cfg := &config.Viper{ + Path: "configs/.rr-issue659.yaml", + Prefix: "rr", + } + + err = cont.RegisterAll( + cfg, + &rpcPlugin.Plugin{}, + &logger.ZapLogger{}, + &server.Plugin{}, + &httpPlugin.Plugin{}, + ) + assert.NoError(t, err) + + err = cont.Init() + if err != nil { + t.Fatal(err) + } + + ch, err := cont.Serve() + assert.NoError(t, err) + + sig := make(chan os.Signal, 1) + signal.Notify(sig, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) + + wg := &sync.WaitGroup{} + wg.Add(1) + + stopCh := make(chan struct{}, 1) + + go func() { + defer wg.Done() + for { + select { + case e := <-ch: + assert.Fail(t, "error", e.Error.Error()) + err = cont.Stop() + if err != nil { + assert.FailNow(t, "error", err.Error()) + } + case <-sig: + err = cont.Stop() + if err != nil { + assert.FailNow(t, "error", err.Error()) + } + return + case <-stopCh: + // timeout + err = cont.Stop() + if err != nil { + assert.FailNow(t, "error", err.Error()) + } + return + } + } + }() + + time.Sleep(time.Second * 1) + t.Run("HTTPIssue659", echoIssue659) + + stopCh <- struct{}{} + + wg.Wait() +} + +func echoIssue659(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "http://localhost:32552", nil) + assert.NoError(t, err) + + r, err := http.DefaultClient.Do(req) + assert.NoError(t, err) + b, err := ioutil.ReadAll(r.Body) + assert.NoError(t, err) + assert.Empty(t, b) + assert.Equal(t, 500, r.StatusCode) + + err = r.Body.Close() + assert.NoError(t, err) +} + // HELPERS func get(url string) (string, *http.Response, error) { r, err := http.Get(url) //nolint:gosec -- cgit v1.2.3 From 063703e96e5f7cee59139e98d446d5577fb5c224 Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Thu, 3 Jun 2021 16:59:10 +0300 Subject: - Add internal_error_code option - Update tests Signed-off-by: Valery Piashchynski --- tests/plugins/http/configs/.rr-issue659.yaml | 1 + tests/plugins/http/handler_test.go | 56 ++++++++++++++-------------- tests/plugins/http/http_plugin_test.go | 2 +- tests/plugins/http/uploads_test.go | 8 ++-- 4 files changed, 34 insertions(+), 33 deletions(-) (limited to 'tests') diff --git a/tests/plugins/http/configs/.rr-issue659.yaml b/tests/plugins/http/configs/.rr-issue659.yaml index 6e0584a5..bf192fab 100644 --- a/tests/plugins/http/configs/.rr-issue659.yaml +++ b/tests/plugins/http/configs/.rr-issue659.yaml @@ -9,6 +9,7 @@ server: http: address: 127.0.0.1:32552 max_request_size: 1024 + internal_error_code: 444 middleware: [ ] uploads: forbid: [ ".php", ".exe", ".bat" ] diff --git a/tests/plugins/http/handler_test.go b/tests/plugins/http/handler_test.go index 1b7f5bac..40e3a720 100644 --- a/tests/plugins/http/handler_test.go +++ b/tests/plugins/http/handler_test.go @@ -35,7 +35,7 @@ func TestHandler_Echo(t *testing.T) { t.Fatal(err) } - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -66,7 +66,7 @@ func TestHandler_Echo(t *testing.T) { } func Test_HandlerErrors(t *testing.T) { - _, err := handler.NewHandler(1024, config.Uploads{ + _, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, nil) @@ -89,7 +89,7 @@ func TestHandler_Headers(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -150,7 +150,7 @@ func TestHandler_Empty_User_Agent(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -210,7 +210,7 @@ func TestHandler_User_Agent(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -270,7 +270,7 @@ func TestHandler_Cookies(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -335,7 +335,7 @@ func TestHandler_JsonPayload_POST(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -399,7 +399,7 @@ func TestHandler_JsonPayload_PUT(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -459,7 +459,7 @@ func TestHandler_JsonPayload_PATCH(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -519,7 +519,7 @@ func TestHandler_FormData_POST(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -592,7 +592,7 @@ func TestHandler_FormData_POST_Overwrite(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -665,7 +665,7 @@ func TestHandler_FormData_POST_Form_UrlEncoded_Charset(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -737,7 +737,7 @@ func TestHandler_FormData_PUT(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -809,7 +809,7 @@ func TestHandler_FormData_PATCH(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -881,7 +881,7 @@ func TestHandler_Multipart_POST(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -995,7 +995,7 @@ func TestHandler_Multipart_PUT(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -1109,7 +1109,7 @@ func TestHandler_Multipart_PATCH(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -1225,7 +1225,7 @@ func TestHandler_Error(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -1271,7 +1271,7 @@ func TestHandler_Error2(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -1317,7 +1317,7 @@ func TestHandler_Error3(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1, config.Uploads{ + h, err := handler.NewHandler(1, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -1376,7 +1376,7 @@ func TestHandler_ResponseDuration(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -1437,7 +1437,7 @@ func TestHandler_ResponseDurationDelayed(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -1497,7 +1497,7 @@ func TestHandler_ErrorDuration(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) @@ -1572,7 +1572,7 @@ func TestHandler_IP(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, cidrs, p) @@ -1633,7 +1633,7 @@ func TestHandler_XRealIP(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, cidrs, p) @@ -1699,7 +1699,7 @@ func TestHandler_XForwardedFor(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, cidrs, p) @@ -1764,7 +1764,7 @@ func TestHandler_XForwardedFor_NotTrustedRemoteIp(t *testing.T) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, cidrs, p) @@ -1812,7 +1812,7 @@ func BenchmarkHandler_Listen_Echo(b *testing.B) { p.Destroy(context.Background()) }() - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, p) diff --git a/tests/plugins/http/http_plugin_test.go b/tests/plugins/http/http_plugin_test.go index f8044c97..a1fc94ec 100644 --- a/tests/plugins/http/http_plugin_test.go +++ b/tests/plugins/http/http_plugin_test.go @@ -2208,7 +2208,7 @@ func echoIssue659(t *testing.T) { b, err := ioutil.ReadAll(r.Body) assert.NoError(t, err) assert.Empty(t, b) - assert.Equal(t, 500, r.StatusCode) + assert.Equal(t, 444, r.StatusCode) err = r.Body.Close() assert.NoError(t, err) diff --git a/tests/plugins/http/uploads_test.go b/tests/plugins/http/uploads_test.go index 903a930a..df696668 100644 --- a/tests/plugins/http/uploads_test.go +++ b/tests/plugins/http/uploads_test.go @@ -40,7 +40,7 @@ func TestHandler_Upload_File(t *testing.T) { t.Fatal(err) } - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, pool) @@ -123,7 +123,7 @@ func TestHandler_Upload_NestedFile(t *testing.T) { t.Fatal(err) } - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{}, }, nil, pool) @@ -206,7 +206,7 @@ func TestHandler_Upload_File_NoTmpDir(t *testing.T) { t.Fatal(err) } - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: "-------", Forbid: []string{}, }, nil, pool) @@ -289,7 +289,7 @@ func TestHandler_Upload_File_Forbids(t *testing.T) { t.Fatal(err) } - h, err := handler.NewHandler(1024, config.Uploads{ + h, err := handler.NewHandler(1024, 500, config.Uploads{ Dir: os.TempDir(), Forbid: []string{".go"}, }, nil, pool) -- cgit v1.2.3