[Freeipa-devel] [PATCH] 0062 Don't crash when server returns extra output

[Petr Viktorin]

On 06/13/2012 11:40 PM, Rob Crittenden wrote:
> Petr Viktorin wrote:
>> On 06/12/2012 02:38 PM, Simo Sorce wrote:
>>> On Tue, 2012-06-12 at 13:12 +0200, Petr Viktorin wrote:
>>>> This will make older clients usable if new output items get added to
>>>> commands.
>>>>
>>>> Since there might be important information in the extra output, it's
>>>> not
>>>> ignored as the ticket asks. Instead it's printed, but not formatted
>>>> nicely as the client doesn't have enough info for that.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/1721
>>>
>>> Patch is missing.
>>>
>>> Simo.
>>>
>>
>> My apologies
>
> I'd replace the print_line with print_indented so the output looks a
> little nicer.
>
> This sure does make an impression. It looks something like this (with
> print_indented):
>
> $ ipa user-show admin
> User login: admin
> Last name: Administrator
> Home directory: /home/admin
> Login shell: /bin/bash
> UID: 1872200000
> GID: 1872200000
> Account disabled: False
> Password: True
> Member of groups: admins, trust admins
> Kerberos keys available: True
> ------------------------------
> Unexpected output from server:
> ------------------------------
> new: new
>
> It's hard to argue with this as being descriptive it just seems a bit
> overbearing.
>
> I have a couple of ideas on this.
>
> 1. We could detect and supress unexpected output by default and include
> a note at the end, something like:
>
> Unexpected output suppressed, use --all to show.

That would work with show/find, but you can't just re-run add/mod commands.

> 2. Replace the print_dashed with print_line and embed a \n in the value
> so it would look like:
>
> $ ipa user-show admin
> User login: admin
> Last name: Administrator
> Home directory: /home/admin
> Login shell: /bin/bash
> UID: 1872200000
> GID: 1872200000
> Account disabled: False
> Password: True
> Member of groups: admins, trust admins
> Kerberos keys available: True
>
> Unexpected output from server:
> new: new

I went with an extra print instead of the '\n' (it won't confuse 
translators as much).

I also now use print_plain instead of print_line, so the output doesn't 
get truncated.

> I think we'll need to document this somewhere in any case, explaining
> how this situation can happen. I think it could be very confusing.

I added a “please upgrade” message at the end. That should make the 
situation clear.

> Functionally it works pretty well.
>
> rob


-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0062-02-Don-t-crash-when-server-returns-extra-output.patch
Type: text/x-patch
Size: 8134 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120614/eea0536e/attachment.bin>