flappclient assumes input from stdin will not be unicode

[zancas]

I've registered an application with a flappserver with the --accept-stdin flag. I then invoked the application with unicode type arguments. The flappserver behaves in a difficult to understand manner.

I've traced the data that I input (the unicode) as far as the appserver.services.Command.remote_feed_stdin method at which point it was still passing the data around. At some point subsequent to that the data was converted to ''.

This patch zooko wrote should make the client much pickier about the data types it can receive.

diff --git a/foolscap/appserver/client.py b/foolscap/appserver/client.py
index 44e6671..18ed0da 100644
--- a/foolscap/appserver/client.py
+++ b/foolscap/appserver/client.py
@@ -103,6 +103,8 @@ class RunCommand(Referenceable, Protocol):
         return self.d
 
     def dataReceived(self, data):
+        if not isinstance(data, str):
+            usage.error('stdin can accept only strings of bytes, not "%s"' % (type(data),))
         # this is from stdin. It shouldn't be called until after _started
         # sets up stdio and self.stdin_writer
         self.stdin_writer.callRemoteOnly("feed_stdin", data)
diff --git a/foolscap/appserver/services.py b/foolscap/appserver/services.py
index f75248e..749ef70 100644
--- a/foolscap/appserver/services.py
+++ b/foolscap/appserver/services.py
@@ -235,6 +235,8 @@ class Command(Referenceable):
         self.log_stdin = log_stdin
         self.closed = False
     def remote_feed_stdin(self, data):
+        if not isinstance(data, str):
+            usage.error('stdin can accept only strings of bytes, not "%s"' % (type(data),))
         if self.log_stdin:
             log.msg("stdin: %r" % data)
         self.process.write(data)

[Zooko]

Zancas: I don't think you clearly explained what the problem was. The behavior was that the flappserver silently discarded the data, so that the flapp running inside the flappserver, when reading from its stdin, would get an empty string returned (i.e., there were 0 bytes to read on its stdin). There was no error message or warning logged by the flappserver or sent to the flappclient.

So, the bug is: when you send a unicode to a flappserver as the argument to its "remote_feed_stdin" method, then instead of giving you an error, or coercing the unicode into a stream of bytes, it instead silently discards it.

[Zooko]

Oh I see. Zancas wrote: "At some point subsequent to that the data was converted to . ". Meaning that the data was discarded. Too clever! Brian probably thought that was a typo or something. ☺

[Brian Warner]

Good idea, I'll apply that patch. In the future, could you include it as an attachment instead of inline? That'd make it easier for me to extract and apply.

[Brian Warner]

Patch landed in [a4ee2fb]. Do we need anything else for this, or just the clear error message? Please close this ticket if you think the error message is enough.

[Zooko]

Brian: do you think someone (maybe zancas) should write a unit test for this patch?

[Brian Warner]

Nope, I'm going to pretend that this falls into the realm of precondition checks, so the usage.error is effectively an AssertionError, and I'm ok with not having test coverage of those, especially when I'm in a mood to close out tickets :).