fsim command owner module Stdout and Stderr not working

[golthitarun]

Nature of the bug

Even after setting Stdout in command owner module in fsim, stdout from the device is not written to that writer.

Steps taken to find bug

What did you do to trigger the bug?

Tried sending a command from example server code, but received no response back even when the command was successful and Stdout writer is set.

I figured out the issue is with how bufio.Reader buffered() method is used here https://github.com/fido-device-onboard/go-fdo/blob/bf5656886cbd84339011b1fae221e95953292e8e/fsim/command_device.go#L195

This is skipping all the messages, because buffered() method returns the number of bytes it's read into its buffer, not the number of bytes that are in the underlying io.Reader. Here, its always returning 0.

There is an issue with the test case testing this whole flow too, here https://github.com/fido-device-onboard/go-fdo/blob/bf5656886cbd84339011b1fae221e95953292e8e/fsim/fsim_test.go#L303

This is successful even when the response stdout is empty since its checking if run.outbuf.String() is present in " UTC ".

Steps to ensure bug is gone

I am still trying to fix it, if i am writing the msg data to the writer skipping the condition, its failing here https://github.com/fido-device-onboard/go-fdo/blob/bf5656886cbd84339011b1fae221e95953292e8e/fsim/command_owner.go#L78 , with error errors.errorString=&{unmarshal did not consume all data, had extra 5 bytes: 45 54 20 2f 0a})

How can a fix be validated?

Optional: Reproduction

Do you have an exact list of commands for others to reproduce the problem?

Add "fdo.command" in example server and client, run server and client with -echo-commands enabled.

Client Debug Logs

Attach large files, but place the most relevant details here

Server Debug Logs

Attach large files, but place the most relevant details here

[ben-krieger]

Sorry for not seeing this sooner. My notification settings were not correct.

Reproduce:

diff --git a/examples/cmd/server.go b/examples/cmd/server.go
index d1696cd..d88c8c5 100644
--- a/examples/cmd/server.go
+++ b/examples/cmd/server.go
@@ -569,6 +569,18 @@ func ownerModules(ctx context.Context, guid protocol.GUID, info string, chain []
                                }
                        }
                }
+
+               if slices.Contains(modules, "fdo.command") {
+                       slog.Info("Will run `date --utc` on client")
+                       if !yield("fdo.command", &fsim.RunCommand{
+                               Command: "date",
+                               Args:    []string{"--utc"},
+                               Stdout:  os.Stdout,
+                               Stderr:  os.Stderr,
+                       }) {
+                               return
+                       }
+               }
        }
 }

Server:

➤ go run ./examples/cmd server -db db.test
[2024-10-17 14:44:30] INFO: Listening
  local: 127.0.0.1:8080
  external: localhost:8080
[2024-10-17 14:44:35] INFO: Will run `date --utc` on client

Client:

➤ go run ./examples/cmd client -di http://localhost:8080
➤ go run ./examples/cmd client -echo-commands
Success

[ben-krieger]

You're 100% correct that the code is broken and the test is broken. There's nothing that ever advances the bufio.Reader. I'm trying to recall what I meant to accomplish with the bufio.Reader.... I think it had to do with avoiding io.EOF when more would still be written.

[ben-krieger]

In #13 the test is now failing as expected.

[ben-krieger]

Found a concerning number of subtle bugs. All fixed now. I hope some more people start looking at the FSIM implementations. They're pretty basic.

@golthitarun You may now try like this:

➤ go run ./examples/cmd server -db db.test -command-date
[2024-10-17 21:43:17] INFO: Listening
  local: 127.0.0.1:8080
  external: localhost:8080
date --utc

^Note the echoed command on stdout above

go-fdo ➤ go run ./examples/cmd client -di http://localhost:8080
go-fdo ➤ go run ./examples/cmd client -echo-commands
Success