assuan_read_from_server() and return value

[Marcus Brinkmann]

Hi Ben,

there seem to be separate issues here:

1) It seems to me that the case you describe can only happen if you
have no inquire callback set, right?  In this case, the error code is
going to be overwritten by assuan_transact anyway.

Frankly, I do not understand why there is a dummy read in that place
anyway.  Comparing this with the code in assuan-buffer.c, where there
is no dummy read after sending the END command (this is the code path
taken if there is an inquire callback), the dummy read does not seem
to be necessary, in fact, in my opinion it could be downright harmful.
OTOH, as there is an error code returned by assuan_transact, one could
argue that the connection is in a messed up state anyway at that point
due to the missing inquire callback.  The code may be designed to
salvage from the connection whatever is possible.  But, this fails if
for example after sending END and doing a dummy read the server
command is still not yet finished.

It seems to me the right way to fix this is to send a CANCEL instead
or following the END.

2) However, the function _assuan_read_from_server is also used in two
other places in libassuan, where an error could also be returned due
to a missing handshake.  In this case, the return code could be
useful.  I think it is reasonable to assume that it is a gpg error or
assuan error code at that point, because the low-level handshake
protocol is defined by assuan and we control the sender and receiver
side for this.  In fact, we can always just return whatever ERR number
is provided by the server: assuan_error_t is defined in that way (some
values are reserved for the assuan protocol, the rest is free.  We
recently introduced support for gpg-error numbers, which makes things
a bit more delicate, but I am willing to ignore this complication for
the sake of discussion :).

So, I think I would still like to apply your patch, although it should
not make any difference in the scenario you describe.  Plus, I would
like to replace the dummy read and possibly (if it works) the sending
of END by a sending of CANCEL.

Does this make sense?  I might be missing something here.

Thanks,
Marcus

At Fri, 23 Nov 2007 14:01:04 -0500,
Ben Kibbey <bjk at luxsci.net> wrote:
> 
> I'm using assuan_transact() to process an inquire command. After the
> command is finished, assuan_transact() sends "END" then reads the next
> line from the server via assuan_read_from_server().
> assuan_read_from_server() will parse an ERR line, but won't do anything
> with the error code. Attached is a patch that'll return the server error
> code. Doesn't seem to break anything.
> 
> Index: assuan-client.c
> ===================================================================
> --- assuan-client.c	(revision 278)
> +++ assuan-client.c	(working copy)
> @@ -86,6 +86,7 @@
>        *off = 3;
>        while (line[*off] == ' ')
>          ++*off;
> +      rc = atoi(line+ (*off));
>      }  
>    else if (linelen >= 7
>             && line[0] == 'I' && line[1] == 'N' && line[2] == 'Q'
> 
> -- 
> Benjamin J. Kibbey bjk at luxsci.net/jabber/freenode
> 3019 F5FC AA33 5BC7 BE9F 09D2 393E DBD2 40D5 FA7E
> 
> _______________________________________________
> Gnupg-devel mailing list
> Gnupg-devel at gnupg.org
> http://lists.gnupg.org/mailman/listinfo/gnupg-devel
> 
At Sun, 2 Dec 2007 13:04:22 -0500,
Ben Kibbey <bjk at luxsci.net> wrote:
> 
> On Fri, Nov 23, 2007 at 02:01:04PM -0500, Ben Kibbey wrote:
> > I'm using assuan_transact() to process an inquire command. After the
> > command is finished, assuan_transact() sends "END" then reads the next
> > line from the server via assuan_read_from_server().
> > assuan_read_from_server() will parse an ERR line, but won't do anything
> > with the error code. Attached is a patch that'll return the server error
> > code. Doesn't seem to break anything.
> 
> Oops. The patch does break things. The server isn't guaranteed to return
> a ASSUAN_N or GPG_ERR_N code. And there isn't a way to tell which type
> of error it is. So would it be better to patch the server (pinentry in
> this case) to return gpg-error codes, or do something different with my
> return codes from the server (which does use gpg-error codes)?
> 
> It does make sense to me to consider the ERR response code from a server
> command. Any reason not to?
> 
> Thanks,
> 
> -- 
> Benjamin J. Kibbey bjk at luxsci.net/jabber/freenode
> 3019 F5FC AA33 5BC7 BE9F 09D2 393E DBD2 40D5 FA7E
> 
> _______________________________________________
> Gnupg-devel mailing list
> Gnupg-devel at gnupg.org
> http://lists.gnupg.org/mailman/listinfo/gnupg-devel
>