Add the webpage for accessing a web terminal

coder · kylecarbs · Apr 29, 2022 · Apr 26, 2022 · Apr 27, 2022 · Apr 28, 2022
commit cb5ae98b47bd53cf07e7073d4bddcc2ab02f1007
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -1,5 +1,6 @@
 {
   "cSpell.words": [
+    "circbuf",
     "cliflag",
     "cliui",
     "coderd",
@@ -61,8 +62,10 @@
     "unconvert",
     "Untar",
     "VMID",
+    "weblinks",
     "webrtc",
     "xerrors",
+    "xstate",
     "yamux"
   ],
   "emeraldwalk.runonsave": {

diff --git a/agent/agent.go b/agent/agent.go
@@ -18,8 +18,8 @@ import (
 	"sync"
 	"time"
 
+	"github.com/armon/circbuf"
 	"github.com/google/uuid"
-	"github.com/smallnest/ringbuffer"
 
 	gsyslog "github.com/hashicorp/go-syslog"
 	"go.uber.org/atomic"
@@ -417,6 +417,8 @@ func (a *agent) handleSSHSession(session ssh.Session) error {
 func (a *agent) handleReconnectingPTY(ctx context.Context, rawID string, conn net.Conn) {
 	defer conn.Close()
 
+	// The ID format is referenced in conn.go.
+	// <uuid>:<height>:<width>
 	idParts := strings.Split(rawID, ":")
 	if len(idParts) != 3 {
 		a.logger.Warn(ctx, "client sent invalid id format", slog.F("raw-id", rawID))
@@ -429,6 +431,7 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, rawID string, conn ne
 		a.logger.Warn(ctx, "client sent reconnection token that isn't a uuid", slog.F("id", id), slog.Error(err))
 		return
 	}
+	// Parse the initial terminal dimensions.
 	height, err := strconv.Atoi(idParts[1])
 	if err != nil {
 		a.logger.Warn(ctx, "client sent invalid height", slog.F("id", id), slog.F("height", idParts[1]))
@@ -454,41 +457,55 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, rawID string, conn ne
 			a.logger.Warn(ctx, "create reconnecting pty command", slog.Error(err))
 			return
 		}
-		ptty, _, err := pty.Start(cmd)
+		cmd.Env = append(cmd.Env, "TERM=xterm-256color")
+
+		ptty, process, err := pty.Start(cmd)
 		if err != nil {
 			a.logger.Warn(ctx, "start reconnecting pty command", slog.F("id", id))
 		}
 
+		// Default to buffer 64KB.
+		circularBuffer, err := circbuf.NewBuffer(64 * 1024)
+		if err != nil {
+			a.logger.Warn(ctx, "create circular buffer", slog.Error(err))
+			return
+		}
+
 		a.closeMutex.Lock()
 		a.connCloseWait.Add(1)
 		a.closeMutex.Unlock()
+		ctx, cancelFunc := context.WithCancel(ctx)
 		rpty = &reconnectingPTY{
 			activeConns: make(map[string]net.Conn),
 			ptty:        ptty,
-			timeout:     time.NewTimer(a.reconnectingPTYTimeout),
-			// Default to buffer 1MB.
-			ringBuffer: ringbuffer.New(1 << 20),
+			// Timeouts created with an after func can be reset!
+			timeout:        time.AfterFunc(a.reconnectingPTYTimeout, cancelFunc),
+			circularBuffer: circularBuffer,
 		}
 		a.reconnectingPTYs.Store(id, rpty)
 		go func() {
-			// Close if the inactive timeout occurs, or the context ends.
-			select {
-			case <-rpty.timeout.C:
-				a.logger.Info(ctx, "killing reconnecting pty due to inactivity", slog.F("id", id))
-			case <-ctx.Done():
-			}
+			// When the context has been completed either:
+			// 1. The timeout completed.
+			// 2. The parent context was cancelled.
+			<-ctx.Done()
+			_ = process.Kill()
+		}()
+		go func() {
+			// If the process dies randomly, we should
+			// close the pty.
+			_, _ = process.Wait()
 			rpty.Close()
 		}()
 		go func() {
-			buffer := make([]byte, 32*1024)
+			buffer := make([]byte, 1024)
 			for {
 				read, err := rpty.ptty.Output().Read(buffer)
 				if err != nil {
-					rpty.Close()
+					// When the PTY is closed, this is triggered.
 					break
 				}
 				part := buffer[:read]
-				_, err = rpty.ringBuffer.Write(part)
+				_, err = rpty.circularBuffer.Write(part)
 				if err != nil {
 					a.logger.Error(ctx, "reconnecting pty write buffer", slog.Error(err), slog.F("id", id))
 					break
@@ -499,27 +516,56 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, rawID string, conn ne
 				}
 				rpty.activeConnsMutex.Unlock()
 			}
-			// If we break from the loop, the reconnecting PTY ended.
+
+			// Cleanup the process, PTY, and delete it's
+			// ID from memory.
+			_ = process.Kill()
+			rpty.Close()
 			a.reconnectingPTYs.Delete(id)
 			a.connCloseWait.Done()
 		}()
 	}
+	// Resize the PTY to initial height + width.
 	err = rpty.ptty.Resize(uint16(height), uint16(width))
 	if err != nil {
 		// We can continue after this, it's not fatal!
 		a.logger.Error(ctx, "resize reconnecting pty", slog.F("id", id), slog.Error(err))
 	}
-
-	_, err = conn.Write(rpty.ringBuffer.Bytes())
+	// Write any previously stored data for the TTY.
+	_, err = conn.Write(rpty.circularBuffer.Bytes())
 	if err != nil {
 		a.logger.Warn(ctx, "write reconnecting pty buffer", slog.F("id", id), slog.Error(err))
 		return
 	}
 	connectionID := uuid.NewString()
+	// Multiple connections to the same TTY are permitted.
+	// This could easily be used for terminal sharing, but
+	// we do it because it's a nice user experience to
+	// copy/paste a terminal URL and have it _just work_.
 	rpty.activeConnsMutex.Lock()
 	rpty.activeConns[connectionID] = conn
 	rpty.activeConnsMutex.Unlock()
+	// Resetting this timeout prevents the PTY from exiting.
+	rpty.timeout.Reset(a.reconnectingPTYTimeout)
+
+	heartbeat := time.NewTimer(a.reconnectingPTYTimeout / 2)
+	defer heartbeat.Stop()
+	go func() {
+		// Keep updating the activity while this
+		// connection is alive!
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			case <-heartbeat.C:
+			}
+			rpty.timeout.Reset(a.reconnectingPTYTimeout)
+		}
+	}()
 	defer func() {
+		// After this connection ends, remove it from
+		// the PTYs active connections. If it isn't
+		// removed, all PTY data will be sent to it.
 		rpty.activeConnsMutex.Lock()
 		delete(rpty.activeConns, connectionID)
 		rpty.activeConnsMutex.Unlock()
@@ -579,17 +625,20 @@ type reconnectingPTY struct {
 	activeConnsMutex sync.Mutex
 	activeConns      map[string]net.Conn
 
-	ringBuffer *ringbuffer.RingBuffer
-	timeout    *time.Timer
-	ptty       pty.PTY
+	circularBuffer *circbuf.Buffer
+	timeout        *time.Timer
+	ptty           pty.PTY
 }
 
+// Close ends all connections to the reconnecting
+// PTY and clear the circular buffer.
 func (r *reconnectingPTY) Close() {
 	r.activeConnsMutex.Lock()
 	defer r.activeConnsMutex.Unlock()
 	for _, conn := range r.activeConns {
 		_ = conn.Close()
 	}
 	_ = r.ptty.Close()
-	r.ringBuffer.Reset()
+	r.circularBuffer.Reset()
+	r.timeout.Stop()
 }
diff --git a/agent/agent_test.go b/agent/agent_test.go
@@ -133,7 +133,7 @@ func TestAgent(t *testing.T) {
 
 	t.Run("SFTP", func(t *testing.T) {
 		t.Parallel()
-		sshClient, err := setupAgent(t, agent.Metadata{}).SSHClient()
+		sshClient, err := setupAgent(t, agent.Metadata{}, 0).SSHClient()
 		require.NoError(t, err)
 		client, err := sftp.NewClient(sshClient)
 		require.NoError(t, err)
@@ -170,7 +170,7 @@ func TestAgent(t *testing.T) {
 		content := "somethingnice"
 		setupAgent(t, agent.Metadata{
 			StartupScript: "echo " + content + " > " + tempPath,
-		})
+		}, 0)
 		var gotContent string
 		require.Eventually(t, func() bool {
 			content, err := os.ReadFile(tempPath)
@@ -193,7 +193,13 @@ func TestAgent(t *testing.T) {
 
 	t.Run("ReconnectingPTY", func(t *testing.T) {
 		t.Parallel()
-		conn := setupAgent(t, agent.Metadata{})
+		if runtime.GOOS == "windows" {
+			// This might be our implementation, or ConPTY itself.
+			// It's difficult to find extensive tests for it, so
+			// it seems like it could be either.
+			t.Skip("ConPTY appears to be inconsistent on Windows.")
+		}
+		conn := setupAgent(t, agent.Metadata{}, 0)
 		id := uuid.NewString()
 		netConn, err := conn.ReconnectingPTY(id, 100, 100)
 		require.NoError(t, err)
@@ -231,7 +237,7 @@ func TestAgent(t *testing.T) {
 }
 
 func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) *exec.Cmd {
-	agentConn := setupAgent(t, agent.Metadata{})
+	agentConn := setupAgent(t, agent.Metadata{}, 0)
 	listener, err := net.Listen("tcp", "127.0.0.1:0")
 	require.NoError(t, err)
 	go func() {
@@ -260,20 +266,21 @@ func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) *exe
 }
 
 func setupSSHSession(t *testing.T, options agent.Metadata) *ssh.Session {
-	sshClient, err := setupAgent(t, options).SSHClient()
+	sshClient, err := setupAgent(t, options, 0).SSHClient()
 	require.NoError(t, err)
 	session, err := sshClient.NewSession()
 	require.NoError(t, err)
 	return session
 }
 
-func setupAgent(t *testing.T, metadata agent.Metadata) *agent.Conn {
+func setupAgent(t *testing.T, metadata agent.Metadata, ptyTimeout time.Duration) *agent.Conn {
 	client, server := provisionersdk.TransportPipe()
 	closer := agent.New(func(ctx context.Context, logger slog.Logger) (agent.Metadata, *peerbroker.Listener, error) {
 		listener, err := peerbroker.Listen(server, nil)
 		return metadata, listener, err
 	}, &agent.Options{
-		Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug),
+		Logger:                 slogtest.Make(t, nil).Leveled(slog.LevelDebug),
+		ReconnectingPTYTimeout: ptyTimeout,
 	})
 	t.Cleanup(func() {
 		_ = client.Close()

diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -375,7 +375,7 @@ func (api *api) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
 		_ = conn.Close(websocket.StatusNormalClosure, "ended")
 	}()
 	// Accept text connections, because it's more developer friendly.
-	wsNetConn := websocket.NetConn(r.Context(), conn, websocket.MessageText)
+	wsNetConn := websocket.NetConn(r.Context(), conn, websocket.MessageBinary)
 	agentConn, err := api.dialWorkspaceAgent(r, workspaceAgent.ID)
 	if err != nil {
 		return

diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go
@@ -368,7 +368,7 @@ func (c *Client) WorkspaceAgentReconnectingPTY(ctx context.Context, agentID, rec
 		}
 		return nil, readBodyAsError(res)
 	}
-	return websocket.NetConn(ctx, conn, websocket.MessageText), nil
+	return websocket.NetConn(ctx, conn, websocket.MessageBinary), nil
 }
 
 func (c *Client) turnProxyDialer(ctx context.Context, httpClient *http.Client, path string) proxy.Dialer {

diff --git a/go.mod b/go.mod
@@ -43,6 +43,7 @@ require (
 	cdr.dev/slog v1.4.1
 	cloud.google.com/go/compute v1.6.1
 	github.com/AlecAivazis/survey/v2 v2.3.4
+	github.com/armon/circbuf v0.0.0-20190214190532-5111143e8da2
 	github.com/awalterschulze/gographviz v2.0.3+incompatible
 	github.com/bgentry/speakeasy v0.1.0
 	github.com/briandowns/spinner v1.18.1
@@ -92,7 +93,6 @@ require (
 	github.com/pkg/sftp v1.13.4
 	github.com/quasilyte/go-ruleguard/dsl v0.3.19
 	github.com/robfig/cron/v3 v3.0.1
-	github.com/smallnest/ringbuffer v0.0.0-20210227121335-0a58434b36f2
 	github.com/spf13/cobra v1.4.0
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.7.1

diff --git a/go.sum b/go.sum
@@ -182,6 +182,8 @@ github.com/apparentlymart/go-textseg v1.0.0/go.mod h1:z96Txxhf3xSFMPmb5X/1W05FF/
 github.com/apparentlymart/go-textseg/v13 v13.0.0 h1:Y+KvPE1NYz0xl601PVImeQfFyEy6iT90AvPUL1NNfNw=
 github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo=
 github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o=
+github.com/armon/circbuf v0.0.0-20190214190532-5111143e8da2 h1:7Ip0wMmLHLRJdrloDxZfhMm0xrLXZS8+COSu2bXmEQs=
+github.com/armon/circbuf v0.0.0-20190214190532-5111143e8da2/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o=
 github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
 github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY=
 github.com/armon/go-metrics v0.3.0/go.mod h1:zXjbSimjXTd7vOpY8B0/2LpvNvDoXBuplAD+gJD3GYs=
@@ -1493,8 +1495,6 @@ github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrf
 github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
 github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=
 github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
-github.com/smallnest/ringbuffer v0.0.0-20210227121335-0a58434b36f2 h1:co1YnJJ6rDvcodJzcXObchJMfHclIROMulsWObuNfTY=
-github.com/smallnest/ringbuffer v0.0.0-20210227121335-0a58434b36f2/go.mod h1:mXcZNMJHswhQDDJZIjdtJoG97JIwIa/HdcHNM3w15T0=
 github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d h1:zE9ykElWQ6/NYmHa3jpm/yHnI4xSofP+UP6SpjHcSeM=
 github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc=
 github.com/smartystreets/go-aws-auth v0.0.0-20180515143844-0c1422d1fdb9/go.mod h1:SnhjPscd9TpLiy1LpzGSKh3bXCfxxXuqd9xmQJy3slM=

diff --git a/pty/pty_other.go b/pty/pty_other.go
@@ -46,8 +46,8 @@ func (p *otherPty) Resize(height uint16, width uint16) error {
 	p.mutex.Lock()
 	defer p.mutex.Unlock()
 	return pty.Setsize(p.pty, &pty.Winsize{
-		Rows: width,
-		Cols: height,
+		Rows: height,
+		Cols: width,
 	})
 }
 

diff --git a/site/package.json b/site/package.json
@@ -42,6 +42,10 @@
     "react-router-dom": "6.3.0",
     "swr": "1.2.2",
     "xstate": "4.31.0",
+    "xterm-addon-fit": "^0.5.0",
+    "xterm-addon-web-links": "^0.5.1",
+    "xterm-addon-webgl": "^0.11.4",
+    "xterm-for-react": "^1.0.4",
     "yup": "0.32.11"
   },
   "devDependencies": {

diff --git a/site/src/AppRouter.tsx b/site/src/AppRouter.tsx
@@ -17,6 +17,7 @@ import { SettingsPage } from "./pages/SettingsPage/SettingsPage"
 import { CreateWorkspacePage } from "./pages/TemplatesPages/OrganizationPage/TemplatePage/CreateWorkspacePage"
 import { TemplatePage } from "./pages/TemplatesPages/OrganizationPage/TemplatePage/TemplatePage"
 import { TemplatesPage } from "./pages/TemplatesPages/TemplatesPage"
+import { TerminalPage } from "./pages/TerminalPage/TerminalPage"
 import { UsersPage } from "./pages/UsersPage/UsersPage"
 import { WorkspacePage } from "./pages/WorkspacesPage/WorkspacesPage"
 
@@ -115,6 +116,19 @@ export const AppRouter: React.FC = () => (
         <Route path="linked-accounts" element={<LinkedAccountsPage />} />
       </Route>
 
+      <Route path=":username">
+        <Route path=":workspace">
+          <Route
+            path="terminal"
+            element={
+              <RequireAuth>
+                <TerminalPage />
+              </RequireAuth>
+            }
+          />
+        </Route>
+      </Route>
+
       {/* Using path="*"" means "match anything", so this route
         acts like a catch-all for URLs that we don't have explicit
         routes for. */}

diff --git a/site/src/api/index.ts b/site/src/api/index.ts
@@ -91,7 +91,9 @@ export const getOrganizations = async (): Promise<Types.Organization[]> => {
 }
 
 export const getWorkspace = async (organizationID: string, workspaceName: string): Promise<Types.Workspace> => {
-  const response = await axios.get<Types.Workspace>(`/api/v2/organizations/${organizationID}/workspaces/me/${workspaceName}`)
+  const response = await axios.get<Types.Workspace>(
+    `/api/v2/organizations/${organizationID}/workspaces/me/${workspaceName}`,
+  )
   return response.data
 }
 

diff --git a/site/src/api/types.ts b/site/src/api/types.ts
@@ -73,7 +73,7 @@ export interface Workspace {
 
 export interface WorkspaceResource {
   id: string
-  agents: WorkspaceAgent[]
+  agents?: WorkspaceAgent[]
 }
 
 export interface WorkspaceAgent {
@@ -116,7 +116,7 @@ export interface UpdateProfileRequest {
 }
 
 export interface ReconnectingPTYRequest {
-  readonly data: string
-  readonly height: number
-  readonly width: number
+  readonly data?: string
+  readonly height?: number
+  readonly width?: number
 }