From 9472717144766dbebce546d1acd252eff26a76d2 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Thu, 31 Mar 2022 12:58:37 -0500 Subject: [PATCH 1/6] fix: ensure websocket close messages are truncated to 123 bytes It's possible for websocket close messages to be too long, which cause them to silently fail without a proper close message. See error below: ``` 2022-03-31 17:08:34.862 [INFO] (stdlib) "2022/03/31 17:08:34 websocket: failed to marshal close frame: reason string max is 123 but got \"insert provisioner daemon:Cannot encode []database.ProvisionerType into oid 19098 - []database.ProvisionerType must implement Encoder or be converted to a string\" with length 161" ``` --- cli/configssh_test.go | 3 ++- cli/ssh.go | 3 ++- coderd/provisionerdaemons.go | 8 +++---- coderd/workspaceresources.go | 2 +- coderd/ws.go | 40 ++++++++++++++++++++++++++++++++++ coderd/ws_test.go | 30 +++++++++++++++++++++++++ provisioner/terraform/serve.go | 1 + 7 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 coderd/ws.go create mode 100644 coderd/ws_test.go diff --git a/cli/configssh_test.go b/cli/configssh_test.go index 7589dff5c49a4..140cee2dbece7 100644 --- a/cli/configssh_test.go +++ b/cli/configssh_test.go @@ -4,10 +4,11 @@ import ( "os" "testing" + "github.com/stretchr/testify/require" + "github.com/coder/coder/cli/clitest" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/pty/ptytest" - "github.com/stretchr/testify/require" ) func TestConfigSSH(t *testing.T) { diff --git a/cli/ssh.go b/cli/ssh.go index d04986b8e6605..db8395454df2c 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -13,11 +13,12 @@ import ( gossh "golang.org/x/crypto/ssh" "golang.org/x/xerrors" + "golang.org/x/crypto/ssh/terminal" + "github.com/coder/coder/cli/cliflag" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" - "golang.org/x/crypto/ssh/terminal" ) func ssh() *cobra.Command { diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index 89f4cce105724..79a4c24eaa2e7 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -56,7 +56,7 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request Provisioners: []database.ProvisionerType{database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform}, }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmt.Sprintf("insert provisioner daemon:% s", err)) + _ = conn.Close(websocket.StatusInternalError, fmtWebsocketCloseMsg("insert provisioner daemon: %s", err)) return } @@ -67,7 +67,7 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request config.LogOutput = io.Discard session, err := yamux.Server(websocket.NetConn(r.Context(), conn, websocket.MessageBinary), config) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmt.Sprintf("multiplex server: %s", err)) + _ = conn.Close(websocket.StatusInternalError, fmtWebsocketCloseMsg("multiplex server: %s", err)) return } mux := drpcmux.New() @@ -80,13 +80,13 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request Logger: api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)), }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmt.Sprintf("drpc register provisioner daemon: %s", err)) + _ = conn.Close(websocket.StatusInternalError, fmtWebsocketCloseMsg("drpc register provisioner daemon: %s", err)) return } server := drpcserver.New(mux) err = server.Serve(r.Context(), session) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmt.Sprintf("serve: %s", err)) + _ = conn.Close(websocket.StatusInternalError, fmtWebsocketCloseMsg("serve: %s", err)) return } _ = conn.Close(websocket.StatusGoingAway, "") diff --git a/coderd/workspaceresources.go b/coderd/workspaceresources.go index 7342b01ea2d4b..15182f031c3e0 100644 --- a/coderd/workspaceresources.go +++ b/coderd/workspaceresources.go @@ -108,7 +108,7 @@ func (api *api) workspaceResourceDial(rw http.ResponseWriter, r *http.Request) { Pubsub: api.Pubsub, }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmt.Sprintf("serve: %s", err)) + _ = conn.Close(websocket.StatusInternalError, fmtWebsocketCloseMsg("serve: %s", err)) return } } diff --git a/coderd/ws.go b/coderd/ws.go new file mode 100644 index 0000000000000..69e52db69ee74 --- /dev/null +++ b/coderd/ws.go @@ -0,0 +1,40 @@ +package coderd + +import ( + "fmt" + "strings" +) + +const websocketCloseMaxLen = 123 + +// fmtWebsocketCloseMsg formats a websocket close message and ensures it is +// truncated to the maximum allowed length. +func fmtWebsocketCloseMsg(format string, vars ...any) string { + msg := fmt.Sprintf(format, vars...) + + // Cap msg length at 123 bytes. nhooyr/websocket only allows close messages + // of this length. + if len(msg) > websocketCloseMaxLen { + return truncateString(msg, websocketCloseMaxLen) + } + + return msg +} + +// truncateString safely truncates a string to a maximum size of byteLen. It +// writes whole runes until a single rune would increase the string size above +// byteLen. +func truncateString(str string, byteLen int) string { + builder := strings.Builder{} + builder.Grow(byteLen) + + for _, char := range str { + if builder.Len()+len(string(char)) > byteLen { + break + } + + _, _ = builder.WriteRune(char) + } + + return builder.String() +} diff --git a/coderd/ws_test.go b/coderd/ws_test.go new file mode 100644 index 0000000000000..345d9e9403444 --- /dev/null +++ b/coderd/ws_test.go @@ -0,0 +1,30 @@ +// This file tests an internal function. +//nolint:testpackage +package coderd + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_websocketCloseMsg(t *testing.T) { + t.Parallel() + + t.Run("TruncateSingleByteCharacters", func(t *testing.T) { + t.Parallel() + + msg := strings.Repeat("d", 255) + trunc := fmtWebsocketCloseMsg(msg) + assert.LessOrEqual(t, len(trunc), 123) + }) + + t.Run("TruncateMultiByteCharacters", func(t *testing.T) { + t.Parallel() + + msg := strings.Repeat("こんにちは", 10) + trunc := fmtWebsocketCloseMsg(msg) + assert.LessOrEqual(t, len(trunc), 123) + }) +} diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go index 804dbd64131e1..d856508c519b9 100644 --- a/provisioner/terraform/serve.go +++ b/provisioner/terraform/serve.go @@ -10,6 +10,7 @@ import ( "cdr.dev/slog" "github.com/cli/safeexec" + "github.com/coder/coder/provisionersdk" "github.com/hashicorp/hc-install/product" From c3bf51609542d8c856f6e811ba4863f49236f631 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Thu, 31 Mar 2022 13:42:59 -0500 Subject: [PATCH 2/6] move to coderd.go --- coderd/coderd.go | 36 ++++++++++++++++++++++++++++++++ coderd/coderd_test.go | 24 ++++++++++++++++++++++ coderd/provisionerdaemons.go | 8 ++++---- coderd/workspaceresources.go | 2 +- coderd/ws.go | 40 ------------------------------------ coderd/ws_test.go | 30 --------------------------- 6 files changed, 65 insertions(+), 75 deletions(-) delete mode 100644 coderd/ws.go delete mode 100644 coderd/ws_test.go diff --git a/coderd/coderd.go b/coderd/coderd.go index 0eac01793eb76..b999e247dab20 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1,8 +1,10 @@ package coderd import ( + "fmt" "net/http" "net/url" + "strings" "sync" "time" @@ -199,3 +201,37 @@ type api struct { websocketWaitMutex sync.Mutex websocketWaitGroup sync.WaitGroup } + +const websocketCloseMaxLen = 123 + +// fmtWebsocketCloseMsg formats a websocket close message and ensures it is +// truncated to the maximum allowed length. +func FmtWebsocketCloseMsg(format string, vars ...any) string { + msg := fmt.Sprintf(format, vars...) + + // Cap msg length at 123 bytes. nhooyr/websocket only allows close messages + // of this length. + if len(msg) > websocketCloseMaxLen { + // truncateString safely truncates a string to a maximum size of byteLen. It + // writes whole runes until a single rune would increase the string size above + // byteLen. + truncateString := func(str string, byteLen int) string { + builder := strings.Builder{} + builder.Grow(byteLen) + + for _, char := range str { + if builder.Len()+len(string(char)) > byteLen { + break + } + + _, _ = builder.WriteRune(char) + } + + return builder.String() + } + + return truncateString(msg, websocketCloseMaxLen) + } + + return msg +} diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index ef360326b1d8e..472ad9c7b9f78 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -1,11 +1,35 @@ package coderd_test import ( + "strings" "testing" + "github.com/stretchr/testify/assert" "go.uber.org/goleak" + + "github.com/coder/coder/coderd" ) func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } + +func TestFmtWebsocketCloseMsg(t *testing.T) { + t.Parallel() + + t.Run("TruncateSingleByteCharacters", func(t *testing.T) { + t.Parallel() + + msg := strings.Repeat("d", 255) + trunc := coderd.FmtWebsocketCloseMsg(msg) + assert.LessOrEqual(t, len(trunc), 123) + }) + + t.Run("TruncateMultiByteCharacters", func(t *testing.T) { + t.Parallel() + + msg := strings.Repeat("こんにちは", 10) + trunc := coderd.FmtWebsocketCloseMsg(msg) + assert.LessOrEqual(t, len(trunc), 123) + }) +} diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index 79a4c24eaa2e7..e45b07e97493b 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -56,7 +56,7 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request Provisioners: []database.ProvisionerType{database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform}, }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmtWebsocketCloseMsg("insert provisioner daemon: %s", err)) + _ = conn.Close(websocket.StatusInternalError, FmtWebsocketCloseMsg("insert provisioner daemon: %s", err)) return } @@ -67,7 +67,7 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request config.LogOutput = io.Discard session, err := yamux.Server(websocket.NetConn(r.Context(), conn, websocket.MessageBinary), config) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmtWebsocketCloseMsg("multiplex server: %s", err)) + _ = conn.Close(websocket.StatusInternalError, FmtWebsocketCloseMsg("multiplex server: %s", err)) return } mux := drpcmux.New() @@ -80,13 +80,13 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request Logger: api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)), }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmtWebsocketCloseMsg("drpc register provisioner daemon: %s", err)) + _ = conn.Close(websocket.StatusInternalError, FmtWebsocketCloseMsg("drpc register provisioner daemon: %s", err)) return } server := drpcserver.New(mux) err = server.Serve(r.Context(), session) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmtWebsocketCloseMsg("serve: %s", err)) + _ = conn.Close(websocket.StatusInternalError, FmtWebsocketCloseMsg("serve: %s", err)) return } _ = conn.Close(websocket.StatusGoingAway, "") diff --git a/coderd/workspaceresources.go b/coderd/workspaceresources.go index 15182f031c3e0..938facff48bd0 100644 --- a/coderd/workspaceresources.go +++ b/coderd/workspaceresources.go @@ -108,7 +108,7 @@ func (api *api) workspaceResourceDial(rw http.ResponseWriter, r *http.Request) { Pubsub: api.Pubsub, }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmtWebsocketCloseMsg("serve: %s", err)) + _ = conn.Close(websocket.StatusInternalError, FmtWebsocketCloseMsg("serve: %s", err)) return } } diff --git a/coderd/ws.go b/coderd/ws.go deleted file mode 100644 index 69e52db69ee74..0000000000000 --- a/coderd/ws.go +++ /dev/null @@ -1,40 +0,0 @@ -package coderd - -import ( - "fmt" - "strings" -) - -const websocketCloseMaxLen = 123 - -// fmtWebsocketCloseMsg formats a websocket close message and ensures it is -// truncated to the maximum allowed length. -func fmtWebsocketCloseMsg(format string, vars ...any) string { - msg := fmt.Sprintf(format, vars...) - - // Cap msg length at 123 bytes. nhooyr/websocket only allows close messages - // of this length. - if len(msg) > websocketCloseMaxLen { - return truncateString(msg, websocketCloseMaxLen) - } - - return msg -} - -// truncateString safely truncates a string to a maximum size of byteLen. It -// writes whole runes until a single rune would increase the string size above -// byteLen. -func truncateString(str string, byteLen int) string { - builder := strings.Builder{} - builder.Grow(byteLen) - - for _, char := range str { - if builder.Len()+len(string(char)) > byteLen { - break - } - - _, _ = builder.WriteRune(char) - } - - return builder.String() -} diff --git a/coderd/ws_test.go b/coderd/ws_test.go deleted file mode 100644 index 345d9e9403444..0000000000000 --- a/coderd/ws_test.go +++ /dev/null @@ -1,30 +0,0 @@ -// This file tests an internal function. -//nolint:testpackage -package coderd - -import ( - "strings" - "testing" - - "github.com/stretchr/testify/assert" -) - -func Test_websocketCloseMsg(t *testing.T) { - t.Parallel() - - t.Run("TruncateSingleByteCharacters", func(t *testing.T) { - t.Parallel() - - msg := strings.Repeat("d", 255) - trunc := fmtWebsocketCloseMsg(msg) - assert.LessOrEqual(t, len(trunc), 123) - }) - - t.Run("TruncateMultiByteCharacters", func(t *testing.T) { - t.Parallel() - - msg := strings.Repeat("こんにちは", 10) - trunc := fmtWebsocketCloseMsg(msg) - assert.LessOrEqual(t, len(trunc), 123) - }) -} From c3bae1c722372b42bebd4e468eb0cec319d51938 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Thu, 31 Mar 2022 13:47:49 -0500 Subject: [PATCH 3/6] simpler truncating --- coderd/coderd.go | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index b999e247dab20..0ca45e6af1662 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -212,25 +212,9 @@ func FmtWebsocketCloseMsg(format string, vars ...any) string { // Cap msg length at 123 bytes. nhooyr/websocket only allows close messages // of this length. if len(msg) > websocketCloseMaxLen { - // truncateString safely truncates a string to a maximum size of byteLen. It - // writes whole runes until a single rune would increase the string size above - // byteLen. - truncateString := func(str string, byteLen int) string { - builder := strings.Builder{} - builder.Grow(byteLen) - - for _, char := range str { - if builder.Len()+len(string(char)) > byteLen { - break - } - - _, _ = builder.WriteRune(char) - } - - return builder.String() - } - - return truncateString(msg, websocketCloseMaxLen) + // Trim the string to 123 bytes. If we accidentally cut in the middle of + // a UTF-8 character, remove it from the string. + return strings.ToValidUTF8(string(msg[123]), "") } return msg From 82ea13c8e712de6466b8ddee1715695c220d7fff Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Thu, 31 Mar 2022 13:51:38 -0500 Subject: [PATCH 4/6] fix imports --- provisioner/terraform/serve.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go index d856508c519b9..a5fc22c094050 100644 --- a/provisioner/terraform/serve.go +++ b/provisioner/terraform/serve.go @@ -4,17 +4,14 @@ import ( "context" "path/filepath" + "github.com/cli/safeexec" "github.com/hashicorp/go-version" + "github.com/hashicorp/hc-install/product" + "github.com/hashicorp/hc-install/releases" "golang.org/x/xerrors" "cdr.dev/slog" - - "github.com/cli/safeexec" - "github.com/coder/coder/provisionersdk" - - "github.com/hashicorp/hc-install/product" - "github.com/hashicorp/hc-install/releases" ) var ( From ceab62527e61f6d7b9a95f6b8d4eceff05342ed6 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Fri, 1 Apr 2022 12:43:52 -0500 Subject: [PATCH 5/6] httpapi.WebsocketCloseMessage --- coderd/coderd.go | 20 -------------------- coderd/coderd_test.go | 24 ------------------------ coderd/httpapi/httpapi.go | 18 ++++++++++++++++++ coderd/httpapi/httpapi_test.go | 22 ++++++++++++++++++++++ coderd/provisionerdaemons.go | 8 ++++---- coderd/workspaceresources.go | 2 +- 6 files changed, 45 insertions(+), 49 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 0ca45e6af1662..0eac01793eb76 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1,10 +1,8 @@ package coderd import ( - "fmt" "net/http" "net/url" - "strings" "sync" "time" @@ -201,21 +199,3 @@ type api struct { websocketWaitMutex sync.Mutex websocketWaitGroup sync.WaitGroup } - -const websocketCloseMaxLen = 123 - -// fmtWebsocketCloseMsg formats a websocket close message and ensures it is -// truncated to the maximum allowed length. -func FmtWebsocketCloseMsg(format string, vars ...any) string { - msg := fmt.Sprintf(format, vars...) - - // Cap msg length at 123 bytes. nhooyr/websocket only allows close messages - // of this length. - if len(msg) > websocketCloseMaxLen { - // Trim the string to 123 bytes. If we accidentally cut in the middle of - // a UTF-8 character, remove it from the string. - return strings.ToValidUTF8(string(msg[123]), "") - } - - return msg -} diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index 472ad9c7b9f78..ef360326b1d8e 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -1,35 +1,11 @@ package coderd_test import ( - "strings" "testing" - "github.com/stretchr/testify/assert" "go.uber.org/goleak" - - "github.com/coder/coder/coderd" ) func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } - -func TestFmtWebsocketCloseMsg(t *testing.T) { - t.Parallel() - - t.Run("TruncateSingleByteCharacters", func(t *testing.T) { - t.Parallel() - - msg := strings.Repeat("d", 255) - trunc := coderd.FmtWebsocketCloseMsg(msg) - assert.LessOrEqual(t, len(trunc), 123) - }) - - t.Run("TruncateMultiByteCharacters", func(t *testing.T) { - t.Parallel() - - msg := strings.Repeat("こんにちは", 10) - trunc := coderd.FmtWebsocketCloseMsg(msg) - assert.LessOrEqual(t, len(trunc), 123) - }) -} diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go index 52fabee3694da..591d279c0ac72 100644 --- a/coderd/httpapi/httpapi.go +++ b/coderd/httpapi/httpapi.go @@ -115,3 +115,21 @@ func Read(rw http.ResponseWriter, r *http.Request, value interface{}) bool { } return true } + +const websocketCloseMaxLen = 123 + +// WebsocketCloseMsg formats a websocket close message and ensures it is +// truncated to the maximum allowed length. +func WebsocketCloseMsg(format string, vars ...any) string { + msg := fmt.Sprintf(format, vars...) + + // Cap msg length at 123 bytes. nhooyr/websocket only allows close messages + // of this length. + if len(msg) > websocketCloseMaxLen { + // Trim the string to 123 bytes. If we accidentally cut in the middle of + // a UTF-8 character, remove it from the string. + return strings.ToValidUTF8(string(msg[123]), "") + } + + return msg +} diff --git a/coderd/httpapi/httpapi_test.go b/coderd/httpapi/httpapi_test.go index 87ce03f51d36c..593819587e1d5 100644 --- a/coderd/httpapi/httpapi_test.go +++ b/coderd/httpapi/httpapi_test.go @@ -5,8 +5,10 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/httpapi" @@ -142,3 +144,23 @@ func TestReadUsername(t *testing.T) { }) } } + +func WebsocketCloseMsg(t *testing.T) { + t.Parallel() + + t.Run("TruncateSingleByteCharacters", func(t *testing.T) { + t.Parallel() + + msg := strings.Repeat("d", 255) + trunc := httpapi.WebsocketCloseMsg(msg) + assert.LessOrEqual(t, len(trunc), 123) + }) + + t.Run("TruncateMultiByteCharacters", func(t *testing.T) { + t.Parallel() + + msg := strings.Repeat("こんにちは", 10) + trunc := httpapi.WebsocketCloseMsg(msg) + assert.LessOrEqual(t, len(trunc), 123) + }) +} diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index e45b07e97493b..f9696690940ba 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -56,7 +56,7 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request Provisioners: []database.ProvisionerType{database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform}, }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, FmtWebsocketCloseMsg("insert provisioner daemon: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseMsg("insert provisioner daemon: %s", err)) return } @@ -67,7 +67,7 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request config.LogOutput = io.Discard session, err := yamux.Server(websocket.NetConn(r.Context(), conn, websocket.MessageBinary), config) if err != nil { - _ = conn.Close(websocket.StatusInternalError, FmtWebsocketCloseMsg("multiplex server: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseMsg("multiplex server: %s", err)) return } mux := drpcmux.New() @@ -80,13 +80,13 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request Logger: api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)), }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, FmtWebsocketCloseMsg("drpc register provisioner daemon: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseMsg("drpc register provisioner daemon: %s", err)) return } server := drpcserver.New(mux) err = server.Serve(r.Context(), session) if err != nil { - _ = conn.Close(websocket.StatusInternalError, FmtWebsocketCloseMsg("serve: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseMsg("serve: %s", err)) return } _ = conn.Close(websocket.StatusGoingAway, "") diff --git a/coderd/workspaceresources.go b/coderd/workspaceresources.go index 938facff48bd0..e178c94e84333 100644 --- a/coderd/workspaceresources.go +++ b/coderd/workspaceresources.go @@ -108,7 +108,7 @@ func (api *api) workspaceResourceDial(rw http.ResponseWriter, r *http.Request) { Pubsub: api.Pubsub, }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, FmtWebsocketCloseMsg("serve: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseMsg("serve: %s", err)) return } } From c70e4082025263f8de5c6c01dc6f1295c41e251b Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Fri, 1 Apr 2022 13:10:25 -0500 Subject: [PATCH 6/6] msg -> sprintf --- coderd/httpapi/httpapi.go | 4 ++-- coderd/httpapi/httpapi_test.go | 4 ++-- coderd/provisionerdaemons.go | 8 ++++---- coderd/workspaceresources.go | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go index 591d279c0ac72..808530d614e1a 100644 --- a/coderd/httpapi/httpapi.go +++ b/coderd/httpapi/httpapi.go @@ -118,9 +118,9 @@ func Read(rw http.ResponseWriter, r *http.Request, value interface{}) bool { const websocketCloseMaxLen = 123 -// WebsocketCloseMsg formats a websocket close message and ensures it is +// WebsocketCloseSprintf formats a websocket close message and ensures it is // truncated to the maximum allowed length. -func WebsocketCloseMsg(format string, vars ...any) string { +func WebsocketCloseSprintf(format string, vars ...any) string { msg := fmt.Sprintf(format, vars...) // Cap msg length at 123 bytes. nhooyr/websocket only allows close messages diff --git a/coderd/httpapi/httpapi_test.go b/coderd/httpapi/httpapi_test.go index 593819587e1d5..0e7e45881a2bd 100644 --- a/coderd/httpapi/httpapi_test.go +++ b/coderd/httpapi/httpapi_test.go @@ -152,7 +152,7 @@ func WebsocketCloseMsg(t *testing.T) { t.Parallel() msg := strings.Repeat("d", 255) - trunc := httpapi.WebsocketCloseMsg(msg) + trunc := httpapi.WebsocketCloseSprintf(msg) assert.LessOrEqual(t, len(trunc), 123) }) @@ -160,7 +160,7 @@ func WebsocketCloseMsg(t *testing.T) { t.Parallel() msg := strings.Repeat("こんにちは", 10) - trunc := httpapi.WebsocketCloseMsg(msg) + trunc := httpapi.WebsocketCloseSprintf(msg) assert.LessOrEqual(t, len(trunc), 123) }) } diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index f9696690940ba..28f4bb8bb9cf4 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -56,7 +56,7 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request Provisioners: []database.ProvisionerType{database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform}, }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseMsg("insert provisioner daemon: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("insert provisioner daemon: %s", err)) return } @@ -67,7 +67,7 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request config.LogOutput = io.Discard session, err := yamux.Server(websocket.NetConn(r.Context(), conn, websocket.MessageBinary), config) if err != nil { - _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseMsg("multiplex server: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("multiplex server: %s", err)) return } mux := drpcmux.New() @@ -80,13 +80,13 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request Logger: api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)), }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseMsg("drpc register provisioner daemon: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("drpc register provisioner daemon: %s", err)) return } server := drpcserver.New(mux) err = server.Serve(r.Context(), session) if err != nil { - _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseMsg("serve: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("serve: %s", err)) return } _ = conn.Close(websocket.StatusGoingAway, "") diff --git a/coderd/workspaceresources.go b/coderd/workspaceresources.go index e178c94e84333..532fc64ea5c51 100644 --- a/coderd/workspaceresources.go +++ b/coderd/workspaceresources.go @@ -108,7 +108,7 @@ func (api *api) workspaceResourceDial(rw http.ResponseWriter, r *http.Request) { Pubsub: api.Pubsub, }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseMsg("serve: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("serve: %s", err)) return } }