From 72a98d99b61422f5349ba46df9df6d7a85602e2e Mon Sep 17 00:00:00 2001 From: Nikita Kryuchkov Date: Fri, 24 Jan 2020 15:01:00 +0400 Subject: [PATCH] Fix hypervisor handlers behaviour on empty and malformed requests --- pkg/app/mock_rpc_client.go | 3 +- pkg/hypervisor/hypervisor.go | 38 +++++++++++++++++---- pkg/hypervisor/user_manager.go | 32 ++++++++++++++--- pkg/router/mock_router.go | 13 ++++--- pkg/router/route_group_test.go | 5 +-- pkg/router/routerclient/client_test.go | 10 +++--- pkg/visor/rpc_test.go | 1 + vendor/golang.org/x/net/nettest/conntest.go | 6 ++-- 8 files changed, 81 insertions(+), 27 deletions(-) diff --git a/pkg/app/mock_rpc_client.go b/pkg/app/mock_rpc_client.go index b07488b79e..5e3dc70007 100644 --- a/pkg/app/mock_rpc_client.go +++ b/pkg/app/mock_rpc_client.go @@ -3,9 +3,10 @@ package app import ( - appnet "github.com/SkycoinProject/skywire-mainnet/pkg/app/appnet" mock "github.com/stretchr/testify/mock" + appnet "github.com/SkycoinProject/skywire-mainnet/pkg/app/appnet" + routing "github.com/SkycoinProject/skywire-mainnet/pkg/routing" time "time" diff --git a/pkg/hypervisor/hypervisor.go b/pkg/hypervisor/hypervisor.go index 2f0614c75a..1c19dccc49 100644 --- a/pkg/hypervisor/hypervisor.go +++ b/pkg/hypervisor/hypervisor.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "errors" "fmt" + "io" "math/rand" "net" "net/http" @@ -151,7 +152,6 @@ func (m *Node) ServeHTTP(w http.ResponseWriter, req *http.Request) { r.Get("/user", m.users.UserInfo()) r.Post("/change-password", m.users.ChangePassword()) - r.Post("/exec/{pk}", m.exec()) r.Get("/nodes", m.getNodes()) r.Get("/nodes/{pk}/health", m.getHealth()) r.Get("/nodes/{pk}/uptime", m.getUptime()) @@ -172,6 +172,7 @@ func (m *Node) ServeHTTP(w http.ResponseWriter, req *http.Request) { r.Delete("/nodes/{pk}/routes/{rid}", m.deleteRoute()) r.Get("/nodes/{pk}/loops", m.getLoops()) r.Get("/nodes/{pk}/restart", m.restart()) + r.Post("/nodes/{pk}/exec", m.exec()) }) }) @@ -239,7 +240,12 @@ func (m *Node) exec() http.HandlerFunc { } if err := httputil.ReadJSON(r, &reqBody); err != nil { - httputil.WriteJSON(w, r, http.StatusBadRequest, err) + if err != io.EOF { + log.Warnf("exec request: %v", err) + } + + httputil.WriteJSON(w, r, http.StatusBadRequest, ErrMalformedRequest) + return } @@ -337,7 +343,12 @@ func (m *Node) putApp() http.HandlerFunc { } if err := httputil.ReadJSON(r, &reqBody); err != nil { - httputil.WriteJSON(w, r, http.StatusBadRequest, err) + if err != io.EOF { + log.Warnf("putApp request: %v", err) + } + + httputil.WriteJSON(w, r, http.StatusBadRequest, ErrMalformedRequest) + return } @@ -472,7 +483,12 @@ func (m *Node) postTransport() http.HandlerFunc { } if err := httputil.ReadJSON(r, &reqBody); err != nil { - httputil.WriteJSON(w, r, http.StatusBadRequest, err) + if err != io.EOF { + log.Warnf("postTransport request: %v", err) + } + + httputil.WriteJSON(w, r, http.StatusBadRequest, ErrMalformedRequest) + return } @@ -550,7 +566,12 @@ func (m *Node) postRoute() http.HandlerFunc { return m.withCtx(m.nodeCtx, func(w http.ResponseWriter, r *http.Request, ctx *httpCtx) { var summary routing.RuleSummary if err := httputil.ReadJSON(r, &summary); err != nil { - httputil.WriteJSON(w, r, http.StatusBadRequest, err) + if err != io.EOF { + log.Warnf("postRoute request: %v", err) + } + + httputil.WriteJSON(w, r, http.StatusBadRequest, ErrMalformedRequest) + return } @@ -591,7 +612,12 @@ func (m *Node) putRoute() http.HandlerFunc { return m.withCtx(m.routeCtx, func(w http.ResponseWriter, r *http.Request, ctx *httpCtx) { var summary routing.RuleSummary if err := httputil.ReadJSON(r, &summary); err != nil { - httputil.WriteJSON(w, r, http.StatusBadRequest, err) + if err != io.EOF { + log.Warnf("putRoute request: %v", err) + } + + httputil.WriteJSON(w, r, http.StatusBadRequest, ErrMalformedRequest) + return } diff --git a/pkg/hypervisor/user_manager.go b/pkg/hypervisor/user_manager.go index 2507aad369..afb0913350 100644 --- a/pkg/hypervisor/user_manager.go +++ b/pkg/hypervisor/user_manager.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "net/http" "sync" "time" @@ -20,10 +21,11 @@ const ( // Errors associated with user management. var ( - ErrBadBody = errors.New("ill-formatted request body") + ErrNotLoggedIn = errors.New("not logged in") ErrNotLoggedOut = errors.New("not logged out") ErrBadLogin = errors.New("incorrect username or password") ErrBadSession = errors.New("session cookie is either non-existent, expired, or ill-formatted") + ErrMalformedRequest = errors.New("request format is malformed") ErrBadUsernameFormat = errors.New("format of 'username' is not accepted") ErrUserNotFound = errors.New("user is either deleted or not found") ) @@ -77,7 +79,17 @@ func (s *UserManager) Login() http.HandlerFunc { } if err := httputil.ReadJSON(r, &rb); err != nil { - httputil.WriteJSON(w, r, http.StatusBadRequest, ErrBadBody) + if err != io.EOF { + log.Warnf("Login request: %v", err) + } + + httputil.WriteJSON(w, r, http.StatusBadRequest, ErrMalformedRequest) + + return + } + + if !checkUsernameFormat(rb.Username) { + httputil.WriteJSON(w, r, http.StatusBadRequest, ErrBadUsernameFormat) return } @@ -115,7 +127,7 @@ func (s *UserManager) Login() http.HandlerFunc { func (s *UserManager) Logout() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if err := s.delSession(w, r); err != nil { - httputil.WriteJSON(w, r, http.StatusBadRequest, errors.New("not logged in")) + httputil.WriteJSON(w, r, http.StatusBadRequest, ErrNotLoggedIn) return } @@ -149,7 +161,12 @@ func (s *UserManager) ChangePassword() http.HandlerFunc { } if err := httputil.ReadJSON(r, &rb); err != nil { - httputil.WriteJSON(w, r, http.StatusBadRequest, err) + if err != io.EOF { + log.Warnf("ChangePassword request: %v", err) + } + + httputil.WriteJSON(w, r, http.StatusBadRequest, ErrMalformedRequest) + return } @@ -185,7 +202,12 @@ func (s *UserManager) CreateAccount() http.HandlerFunc { } if err := httputil.ReadJSON(r, &rb); err != nil { - httputil.WriteJSON(w, r, http.StatusBadRequest, err) + if err != io.EOF { + log.Warnf("CreateAccount request: %v", err) + } + + httputil.WriteJSON(w, r, http.StatusBadRequest, ErrMalformedRequest) + return } diff --git a/pkg/router/mock_router.go b/pkg/router/mock_router.go index 4fde7f9ebc..8907f08fb5 100644 --- a/pkg/router/mock_router.go +++ b/pkg/router/mock_router.go @@ -2,10 +2,15 @@ package router -import cipher "github.com/SkycoinProject/dmsg/cipher" -import context "context" -import mock "github.com/stretchr/testify/mock" -import routing "github.com/SkycoinProject/skywire-mainnet/pkg/routing" +import ( + context "context" + + cipher "github.com/SkycoinProject/dmsg/cipher" + + mock "github.com/stretchr/testify/mock" + + routing "github.com/SkycoinProject/skywire-mainnet/pkg/routing" +) // MockRouter is an autogenerated mock type for the Router type type MockRouter struct { diff --git a/pkg/router/route_group_test.go b/pkg/router/route_group_test.go index 6c2cf39acd..df3491a434 100644 --- a/pkg/router/route_group_test.go +++ b/pkg/router/route_group_test.go @@ -11,12 +11,13 @@ import ( "time" "github.com/SkycoinProject/dmsg/cipher" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/SkycoinProject/skywire-mainnet/pkg/routing" "github.com/SkycoinProject/skywire-mainnet/pkg/snet/snettest" "github.com/SkycoinProject/skywire-mainnet/pkg/snet/stcp" "github.com/SkycoinProject/skywire-mainnet/pkg/transport" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestNewRouteGroup(t *testing.T) { diff --git a/pkg/router/routerclient/client_test.go b/pkg/router/routerclient/client_test.go index 949c85e79a..a2d823e411 100644 --- a/pkg/router/routerclient/client_test.go +++ b/pkg/router/routerclient/client_test.go @@ -6,15 +6,13 @@ import ( "net/rpc" "testing" - "github.com/SkycoinProject/skywire-mainnet/internal/testhelpers" - "github.com/SkycoinProject/dmsg/cipher" - "github.com/SkycoinProject/skywire-mainnet/pkg/routing" - - "github.com/SkycoinProject/skywire-mainnet/pkg/router" - "github.com/stretchr/testify/require" "golang.org/x/net/nettest" + + "github.com/SkycoinProject/skywire-mainnet/internal/testhelpers" + "github.com/SkycoinProject/skywire-mainnet/pkg/router" + "github.com/SkycoinProject/skywire-mainnet/pkg/routing" ) func TestClient_Close(t *testing.T) { diff --git a/pkg/visor/rpc_test.go b/pkg/visor/rpc_test.go index f80ac9baea..2a33facd7c 100644 --- a/pkg/visor/rpc_test.go +++ b/pkg/visor/rpc_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/SkycoinProject/skycoin/src/util/logging" + "github.com/SkycoinProject/skywire-mainnet/internal/testhelpers" "github.com/SkycoinProject/skywire-mainnet/pkg/router" "github.com/SkycoinProject/skywire-mainnet/pkg/util/pathutil" diff --git a/vendor/golang.org/x/net/nettest/conntest.go b/vendor/golang.org/x/net/nettest/conntest.go index 0427987e7d..39cc6a631e 100644 --- a/vendor/golang.org/x/net/nettest/conntest.go +++ b/vendor/golang.org/x/net/nettest/conntest.go @@ -37,9 +37,9 @@ func TestConn(t *testing.T, mp MakePipe) { t.Run("WriteTimeout", func(t *testing.T) { timeoutWrapper(t, mp, testWriteTimeout) }) t.Run("PastTimeout", func(t *testing.T) { timeoutWrapper(t, mp, testPastTimeout) }) t.Run("PresentTimeout", func(t *testing.T) { timeoutWrapper(t, mp, testPresentTimeout) }) - //t.Run("FutureTimeout", func(t *testing.T) { timeoutWrapper(t, mp, testFutureTimeout) }) - //t.Run("CloseTimeout", func(t *testing.T) { timeoutWrapper(t, mp, testCloseTimeout) }) - //t.Run("ConcurrentMethods", func(t *testing.T) { timeoutWrapper(t, mp, testConcurrentMethods) }) + t.Run("FutureTimeout", func(t *testing.T) { timeoutWrapper(t, mp, testFutureTimeout) }) + t.Run("CloseTimeout", func(t *testing.T) { timeoutWrapper(t, mp, testCloseTimeout) }) + t.Run("ConcurrentMethods", func(t *testing.T) { timeoutWrapper(t, mp, testConcurrentMethods) }) } type connTester func(t *testing.T, c1, c2 net.Conn)