From 66b47a8068235f653b3270b0e4d081880d986125 Mon Sep 17 00:00:00 2001 From: Yoshiki Nakagawa Date: Tue, 6 Nov 2018 11:28:51 +0900 Subject: [PATCH 1/5] feat(server): Implements RunFd method (#1609) --- gin.go | 18 ++++++++++++++++++ gin_integration_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/gin.go b/gin.go index 440519f5..b7c77e1f 100644 --- a/gin.go +++ b/gin.go @@ -5,6 +5,7 @@ package gin import ( + "fmt" "html/template" "net" "net/http" @@ -321,6 +322,23 @@ func (engine *Engine) RunUnix(file string) (err error) { return } +// RunFd attaches the router to a http.Server and starts listening and serving HTTP requests +// through the specified file descriptor. +// Note: this method will block the calling goroutine indefinitely unless an error happens. +func (engine *Engine) RunFd(fd int) (err error) { + debugPrint("Listening and serving HTTP on fd@%d", fd) + defer func() { debugPrintError(err) }() + + f := os.NewFile(uintptr(fd), fmt.Sprintf("fd@%d", fd)) + listener, err := net.FileListener(f) + if err != nil { + return + } + defer listener.Close() + err = http.Serve(listener, engine) + return +} + // ServeHTTP conforms to the http.Handler interface. func (engine *Engine) ServeHTTP(w http.ResponseWriter, req *http.Request) { c := engine.pool.Get().(*Context) diff --git a/gin_integration_test.go b/gin_integration_test.go index 038c8b7c..e14a688c 100644 --- a/gin_integration_test.go +++ b/gin_integration_test.go @@ -134,6 +134,42 @@ func TestBadUnixSocket(t *testing.T) { assert.Error(t, router.RunUnix("#/tmp/unix_unit_test")) } +func TestFileDescriptor(t *testing.T) { + router := New() + + addr, err := net.ResolveTCPAddr("tcp", ":8000") + assert.NoError(t, err) + listener, err := net.ListenTCP("tcp", addr) + assert.NoError(t, err) + socketFile, err := listener.File() + assert.NoError(t, err) + + go func() { + router.GET("/example", func(c *Context) { c.String(http.StatusOK, "it worked") }) + assert.NoError(t, router.RunFd(int(socketFile.Fd()))) + }() + // have to wait for the goroutine to start and run the server + // otherwise the main thread will complete + time.Sleep(5 * time.Millisecond) + + c, err := net.Dial("tcp", "localhost:8000") + assert.NoError(t, err) + + fmt.Fprintf(c, "GET /example HTTP/1.0\r\n\r\n") + scanner := bufio.NewScanner(c) + var response string + for scanner.Scan() { + response += scanner.Text() + } + assert.Contains(t, response, "HTTP/1.0 200", "should get a 200") + assert.Contains(t, response, "it worked", "resp body should match") +} + +func TestBadFileDescriptor(t *testing.T) { + router := New() + assert.Error(t, router.RunFd(0)) +} + func TestWithHttptestWithAutoSelectedPort(t *testing.T) { router := New() router.GET("/example", func(c *Context) { c.String(http.StatusOK, "it worked") }) From 37854ee10f7161843c338d9a9fa87f9398aacbbe Mon Sep 17 00:00:00 2001 From: Justin Israel Date: Tue, 6 Nov 2018 18:40:20 +1300 Subject: [PATCH 2/5] Fix panic stack trace being printed during recovery of broken pipe (#1089) (#1259) --- recovery.go | 40 ++++++++++++++++++++++++++++++++-------- recovery_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/recovery.go b/recovery.go index 6c28b4fa..e788cc47 100644 --- a/recovery.go +++ b/recovery.go @@ -10,9 +10,12 @@ import ( "io" "io/ioutil" "log" + "net" "net/http" "net/http/httputil" + "os" "runtime" + "syscall" "time" ) @@ -37,16 +40,37 @@ func RecoveryWithWriter(out io.Writer) HandlerFunc { return func(c *Context) { defer func() { if err := recover(); err != nil { - if logger != nil { - stack := stack(3) - if IsDebugging() { - httprequest, _ := httputil.DumpRequest(c.Request, false) - logger.Printf("[Recovery] %s panic recovered:\n%s\n%s\n%s%s", timeFormat(time.Now()), string(httprequest), err, stack, reset) - } else { - logger.Printf("[Recovery] %s panic recovered:\n%s\n%s%s", timeFormat(time.Now()), err, stack, reset) + // Check for a broken connection, as it is not really a + // condition that warrants a panic stack trace. + var brokenPipe bool + if ne, ok := err.(*net.OpError); ok { + if se, ok := ne.Err.(*os.SyscallError); ok { + if se.Err == syscall.EPIPE || se.Err == syscall.ECONNRESET { + brokenPipe = true + } } } - c.AbortWithStatus(http.StatusInternalServerError) + if logger != nil { + stack := stack(3) + httprequest, _ := httputil.DumpRequest(c.Request, false) + if brokenPipe { + logger.Printf("%s\n%s%s", err, string(httprequest), reset) + } else if IsDebugging() { + logger.Printf("[Recovery] %s panic recovered:\n%s\n%s\n%s%s", + timeFormat(time.Now()), string(httprequest), err, stack, reset) + } else { + logger.Printf("[Recovery] %s panic recovered:\n%s\n%s%s", + timeFormat(time.Now()), err, stack, reset) + } + } + + // If the connection is dead, we can't write a status to it. + if brokenPipe { + c.Error(err.(error)) + c.Abort() + } else { + c.AbortWithStatus(http.StatusInternalServerError) + } } }() c.Next() diff --git a/recovery_test.go b/recovery_test.go index 7d422b74..cafaee91 100644 --- a/recovery_test.go +++ b/recovery_test.go @@ -2,11 +2,16 @@ // Use of this source code is governed by a MIT style // license that can be found in the LICENSE file. +// +build go1.7 + package gin import ( "bytes" + "net" "net/http" + "os" + "syscall" "testing" "github.com/stretchr/testify/assert" @@ -72,3 +77,38 @@ func TestFunction(t *testing.T) { bs := function(1) assert.Equal(t, []byte("???"), bs) } + +// TestPanicWithBrokenPipe asserts that recovery specifically handles +// writing responses to broken pipes +func TestPanicWithBrokenPipe(t *testing.T) { + const expectCode = 204 + + expectMsgs := map[syscall.Errno]string{ + syscall.EPIPE: "broken pipe", + syscall.ECONNRESET: "connection reset", + } + + for errno, expectMsg := range expectMsgs { + t.Run(expectMsg, func(t *testing.T) { + + var buf bytes.Buffer + + router := New() + router.Use(RecoveryWithWriter(&buf)) + router.GET("/recovery", func(c *Context) { + // Start writing response + c.Header("X-Test", "Value") + c.Status(expectCode) + + // Oops. Client connection closed + e := &net.OpError{Err: &os.SyscallError{Err: errno}} + panic(e) + }) + // RUN + w := performRequest(router, "GET", "/recovery") + // TEST + assert.Equal(t, expectCode, w.Code) + assert.Contains(t, buf.String(), expectMsg) + }) + } +} From d6b2c13b18cc0986e579e100bc47f3704a0d3bcd Mon Sep 17 00:00:00 2001 From: Sai Date: Mon, 12 Nov 2018 19:58:24 +0900 Subject: [PATCH 3/5] Fix typo (#1641) --- mode.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mode.go b/mode.go index 6a696329..f787b5ca 100644 --- a/mode.go +++ b/mode.go @@ -17,7 +17,7 @@ const ENV_GIN_MODE = "GIN_MODE" const ( // DebugMode indicates gin mode is debug. DebugMode = "debug" - // ReleaseMode indicates gin mode is relase. + // ReleaseMode indicates gin mode is release. ReleaseMode = "release" // TestMode indicates gin mode is test. TestMode = "test" From 3d44ff82a1fe39b5991f292369eed0d6abf2a430 Mon Sep 17 00:00:00 2001 From: henrylee2cn Date: Thu, 22 Nov 2018 09:07:00 +0800 Subject: [PATCH 4/5] Make sure the debug log contains line breaks (#1650) Many debug logs have no line breaks, so fix them here. - With pull requests: - Open your pull request against `master` - Your pull request should have no more than two commits, if not you should squash them. - It should pass all tests in the available continuous integrations systems such as TravisCI. - You should add/modify tests to cover your proposed code changes. - If your pull request contains a new feature, please document it on the README. --- debug.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/debug.go b/debug.go index c5e65b22..98c67cf7 100644 --- a/debug.go +++ b/debug.go @@ -51,6 +51,9 @@ func debugPrintLoadTemplate(tmpl *template.Template) { func debugPrint(format string, values ...interface{}) { if IsDebugging() { + if !strings.HasSuffix(format, "\n") { + format += "\n" + } fmt.Fprintf(os.Stderr, "[GIN-debug] "+format, values...) } } From 7ec82ee8944579c533425bb6826bcb12b5860ecd Mon Sep 17 00:00:00 2001 From: thinkerou Date: Thu, 22 Nov 2018 09:17:44 +0800 Subject: [PATCH 5/5] recovery: fix issue about syscall import on google app engine (#1640) * recovery: fix issue about syscall import on google app engine * add ToLower() * the whole error message --- recovery.go | 4 ++-- recovery_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/recovery.go b/recovery.go index e788cc47..f06ad56b 100644 --- a/recovery.go +++ b/recovery.go @@ -15,7 +15,7 @@ import ( "net/http/httputil" "os" "runtime" - "syscall" + "strings" "time" ) @@ -45,7 +45,7 @@ func RecoveryWithWriter(out io.Writer) HandlerFunc { var brokenPipe bool if ne, ok := err.(*net.OpError); ok { if se, ok := ne.Err.(*os.SyscallError); ok { - if se.Err == syscall.EPIPE || se.Err == syscall.ECONNRESET { + if strings.Contains(strings.ToLower(se.Error()), "broken pipe") || strings.Contains(strings.ToLower(se.Error()), "connection reset by peer") { brokenPipe = true } } diff --git a/recovery_test.go b/recovery_test.go index cafaee91..c9fb29ce 100644 --- a/recovery_test.go +++ b/recovery_test.go @@ -84,8 +84,8 @@ func TestPanicWithBrokenPipe(t *testing.T) { const expectCode = 204 expectMsgs := map[syscall.Errno]string{ - syscall.EPIPE: "broken pipe", - syscall.ECONNRESET: "connection reset", + syscall.EPIPE: "Broken pipe", + syscall.ECONNRESET: "connection reset by peer", } for errno, expectMsg := range expectMsgs {