Răsfoiți Sursa

Carry a text reply over the control socket as the bytes the command wrote

A text rendering travelled as a JSON string, and a JSON string is UTF-8,
so a byte that was not came out as U+FFFD where the ssh server passes it
through. A certificate's name is whatever the certificate holds. The
reply now carries the rendering as bytes, base64 in the envelope, in a
field of its own; Result is for JSON alone.

- Response.Text, set by the daemon's text rendering; nebula ctl prints
  it as it came
- the client's Text helper, which unquoted the string, goes

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Matt Richardson 1 zi în urmă
părinte
comite
dda59788d0

+ 12 - 13
cmd/internal/ctl/ctl.go

@@ -1,7 +1,6 @@
 package ctl
 
 import (
-	"encoding/json"
 	"errors"
 	"flag"
 	"fmt"
@@ -152,7 +151,7 @@ func Run(args []string) int {
 		return 1
 	}
 
-	out := output(res.Result, req.Format)
+	out := output(res, req.Format)
 	if out != "" {
 		fmt.Fprint(os.Stdout, out)
 		if !strings.HasSuffix(out, "\n") {
@@ -167,7 +166,7 @@ func Run(args []string) int {
 		return 0
 	}
 
-	if len(res.Result) == 0 {
+	if len(res.Result) == 0 && len(res.Text) == 0 {
 		msg := res.Error
 		if !strings.HasSuffix(msg, "\n") {
 			msg += "\n"
@@ -179,22 +178,22 @@ func Run(args []string) int {
 }
 
 // output renders what the daemon answered for the format the line asked for. A text
-// request is answered with the command's rendering carried as a JSON string, and is
-// unquoted back into it; a JSON request is answered with the value itself, which is
-// printed as it came, since a value that happens to be a string is still JSON to the
-// script reading it. Indentation is the client's business: whitespace does not survive a
-// JSON envelope, and the daemon has no reason to care how its answer is laid out.
-func output(result json.RawMessage, format commands.Format) string {
+// request is answered in Text with the bytes the command wrote, printed as they are; a
+// JSON request is answered in Result with the value itself, printed as it came, since a
+// value that happens to be a string is still JSON to the script reading it. Indentation
+// is the client's business: whitespace does not survive a JSON envelope, and the daemon
+// has no reason to care how its answer is laid out.
+func output(res *localapi.Response, format commands.Format) string {
 	switch format {
 	case commands.FormatJSON:
-		return string(result)
+		return string(res.Result)
 	case commands.FormatPrettyJSON:
-		if indented, ok := localapi.Pretty(result); ok {
+		if indented, ok := localapi.Pretty(res.Result); ok {
 			return indented
 		}
-		return string(result)
+		return string(res.Result)
 	default:
-		return localapi.Text(result)
+		return string(res.Text)
 	}
 }
 

+ 9 - 8
cmd/internal/ctl/ctl_test.go

@@ -186,24 +186,25 @@ func TestParseCommandReadsTheLineAsTheDaemonDoes(t *testing.T) {
 }
 
 // TestOutputRendersForTheFormatAsked: what the daemon answered is read by the format the
-// line asked for, never by sniffing the bytes. A text answer is the command's rendering
-// carried as a JSON string and comes back unquoted; a JSON answer whose value is a string
-// stays quoted, since to the script reading it that string is the JSON.
+// line asked for, never by sniffing the bytes. A text answer is the command's rendering,
+// printed as it came; a JSON answer whose value is a string stays quoted, since to the
+// script reading it that string is the JSON.
 func TestOutputRendersForTheFormatAsked(t *testing.T) {
-	text := json.RawMessage(`"Log level is: info\n"`)
+	text := &localapi.Response{Text: []byte("Log level is: info\n")}
 	assert.Equal(t, "Log level is: info\n", output(text, commands.FormatUnset))
 	assert.Equal(t, "Log level is: info\n", output(text, commands.FormatText))
 
-	str := json.RawMessage(`"some string"`)
+	str := &localapi.Response{Result: json.RawMessage(`"some string"`)}
 	assert.Equal(t, `"some string"`, output(str, commands.FormatJSON))
 	assert.Equal(t, `"some string"`, output(str, commands.FormatPrettyJSON))
 
-	obj := json.RawMessage(`{"a":1}`)
+	obj := &localapi.Response{Result: json.RawMessage(`{"a":1}`)}
 	assert.Equal(t, `{"a":1}`, output(obj, commands.FormatJSON))
 	assert.Equal(t, "{\n    \"a\": 1\n}", output(obj, commands.FormatPrettyJSON))
 
-	assert.Empty(t, output(nil, commands.FormatJSON))
-	assert.Empty(t, output(nil, commands.FormatUnset))
+	none := &localapi.Response{}
+	assert.Empty(t, output(none, commands.FormatJSON))
+	assert.Empty(t, output(none, commands.FormatUnset))
 }
 
 // TestSplitLineLiftsOwnFlagsFromAnywhere: -config and -socket are the client's wherever

+ 0 - 22
localapi/client.go

@@ -167,25 +167,3 @@ func Pretty(raw json.RawMessage) (string, bool) {
 
 	return buf.String(), true
 }
-
-// Text renders the Result of a request that asked for text. The daemon carries a text
-// rendering as a JSON string, which is unquoted back into the bytes the command wrote.
-// It is the caller's job to know that text was asked for: a JSON result whose value is
-// a string looks the same on the wire and must not be unquoted, so the choice is made
-// from the request's format, never from the bytes.
-func Text(raw json.RawMessage) string {
-	if len(raw) == 0 {
-		return ""
-	}
-
-	// Only an actual JSON string is unquoted. Unmarshalling into a string also accepts
-	// null, which would turn a `null` answer into no output at all.
-	if raw[0] == '"' {
-		var s string
-		if err := json.Unmarshal(raw, &s); err == nil {
-			return s
-		}
-	}
-
-	return string(raw)
-}

+ 10 - 4
localapi/protocol.go

@@ -71,16 +71,22 @@ type Request struct {
 type Response struct {
 	Version int `json:"version"`
 
-	// Result is what the request produced: the command's output, or the schema when the
-	// request was a Describe. It is a JSON value when the command emitted JSON, so a
-	// caller reads it directly instead of parsing a string that happens to contain JSON,
-	// and a JSON string otherwise.
+	// Result is what a request for JSON produced: the command's value, or the schema
+	// when the request was a Describe, as a JSON value, so a caller reads it directly
+	// instead of parsing a string that happens to contain JSON.
 	//
 	// Whitespace inside a JSON result is not preserved: an envelope normalises it, and
 	// in an API it carries no meaning anyway. Formatting a result for a human to read is
 	// the client's job, which is what Pretty is for.
 	Result json.RawMessage `json:"result,omitempty"`
 
+	// Text is what a request for text produced: the command's own human rendering, the
+	// bytes it wrote and nothing else. They travel as bytes, base64 in the envelope,
+	// rather than as a JSON string, because a string is UTF-8 and the rendering need
+	// not be: a name from a certificate is whatever the certificate holds, and the ssh
+	// server passes it through, so this does too.
+	Text []byte `json:"text,omitempty"`
+
 	// Error explains a failure in a sentence, and is absent on success. It is the only
 	// signal a caller needs: there is no separate status field, because a command that
 	// failed always says why and one that succeeded never does. A command line client

+ 2 - 7
localapi/server.go

@@ -371,13 +371,8 @@ func render(res *Response, cmd *commands.Command, r commands.Result, format comm
 		return res
 	}
 
-	quoted, err := json.Marshal(buf.String())
-	if err != nil {
-		res.Error = err.Error()
-		return res
-	}
-
-	res.Result = quoted
+	// The bytes the command wrote, as they are; see Response.Text.
+	res.Text = buf.Bytes()
 	return res
 }
 

+ 42 - 3
localapi/server_test.go

@@ -93,7 +93,22 @@ func run(t *testing.T, path, command string, args ...string) (string, string) {
 
 	res, err := (&Client{Path: path}).Invoke(command, nil, args, "")
 	require.NoError(t, err)
-	return Text(res.Result), res.Error
+	return textOf(res), res.Error
+}
+
+// textOf is what a test reads as a command's output: the text when text was asked for,
+// else the JSON result, unquoted when it is a JSON string so a test can compare words.
+func textOf(res *Response) string {
+	if len(res.Text) > 0 {
+		return string(res.Text)
+	}
+	if len(res.Result) > 0 && res.Result[0] == '"' {
+		var s string
+		if err := json.Unmarshal(res.Result, &s); err == nil {
+			return s
+		}
+	}
+	return string(res.Result)
 }
 
 func TestServeAllowedCommand(t *testing.T) {
@@ -617,7 +632,7 @@ func TestNamedParametersDriveFlags(t *testing.T) {
 	require.NoError(t, err)
 
 	assert.Empty(t, res.Error)
-	assert.Equal(t, "verbose=true label=here args=[an-operand]", Text(res.Result))
+	assert.Equal(t, "verbose=true label=here args=[an-operand]", textOf(res))
 }
 
 // TestUnknownParameterIsReported: a misspelled parameter is an error rather than a
@@ -735,7 +750,7 @@ func TestPresentationParametersAreRefused(t *testing.T) {
 	res, err := c.Invoke("pair", nil, nil, "text")
 	require.NoError(t, err)
 	require.Empty(t, res.Error)
-	assert.Equal(t, "a is one\n", Text(res.Result))
+	assert.Equal(t, "a is one\n", textOf(res))
 
 	res, err = c.Invoke("pair", nil, nil, "")
 	require.NoError(t, err)
@@ -956,3 +971,27 @@ func TestUnknownRequestFieldIsRefused(t *testing.T) {
 	assert.Contains(t, res.Error, `unknown field "arguments"`)
 	assert.Empty(t, res.Result)
 }
+
+// TestTextOutputKeepsTheBytes: a text rendering travels as the bytes the command wrote.
+// A JSON string would have had to be UTF-8, and a byte that is not would have come out
+// as U+FFFD, where the ssh server passes it through.
+func TestTextOutputKeepsTheBytes(t *testing.T) {
+	reg := commands.New()
+	raw := []byte{'n', 'a', 'm', 'e', ' ', 0xff, 0xfe, '\n'}
+	reg.Register(&commands.Command{
+		Name:             "raw",
+		ShortDescription: "writes what a certificate held",
+		Run:              func(fs any, a []string) (commands.Result, error) { return commands.Value("raw"), nil },
+		Text: func(v any, w io.Writer) error {
+			_, err := w.Write(raw)
+			return err
+		},
+	})
+	_, path := serve(t, reg)
+
+	res, err := (&Client{Path: path}).Invoke("raw", nil, nil, "text")
+	require.NoError(t, err)
+	assert.Empty(t, res.Error)
+	assert.Equal(t, raw, res.Text)
+	assert.Empty(t, res.Result, "text is not also carried as JSON")
+}