mirror of https://github.com/coder/coder.git
fix: ensure websocket close messages are truncated to 123 bytes (#779)
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) <close_notjs.go:72> "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" ```
This commit is contained in:
parent
4601a35c01
commit
dc46ff407b
|
@ -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) {
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -115,3 +115,21 @@ func Read(rw http.ResponseWriter, r *http.Request, value interface{}) bool {
|
|||
}
|
||||
return true
|
||||
}
|
||||
|
||||
const websocketCloseMaxLen = 123
|
||||
|
||||
// WebsocketCloseSprintf formats a websocket close message and ensures it is
|
||||
// truncated to the maximum allowed length.
|
||||
func WebsocketCloseSprintf(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
|
||||
}
|
||||
|
|
|
@ -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.WebsocketCloseSprintf(msg)
|
||||
assert.LessOrEqual(t, len(trunc), 123)
|
||||
})
|
||||
|
||||
t.Run("TruncateMultiByteCharacters", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
msg := strings.Repeat("こんにちは", 10)
|
||||
trunc := httpapi.WebsocketCloseSprintf(msg)
|
||||
assert.LessOrEqual(t, len(trunc), 123)
|
||||
})
|
||||
}
|
||||
|
|
|
@ -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, 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, fmt.Sprintf("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, fmt.Sprintf("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, fmt.Sprintf("serve: %s", err))
|
||||
_ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("serve: %s", err))
|
||||
return
|
||||
}
|
||||
_ = conn.Close(websocket.StatusGoingAway, "")
|
||||
|
|
|
@ -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, httpapi.WebsocketCloseSprintf("serve: %s", err))
|
||||
return
|
||||
}
|
||||
}
|
||||
|
|
|
@ -4,16 +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 (
|
||||
|
|
Loading…
Reference in New Issue