Re: [PATCH] _gpg: Use explicit UIDs for public / secret keys.

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Doron Behar wrote on Wed, Jun 13, 2018 at 18:17:48 +0300:
> On Tue, Jun 12, 2018 at 06:14:11PM -0400, Phil Pennock wrote:
> > Unless you need trust information or some of the specific parts of the
> > userid, using `--fast-list-mode` can have significant wins too.
> 
> I have ran `diff` on the output of `gpg --list-public-keys
> --with-colons` and `gpg --list-public-keys --fast-list-mode
> --with-colons` and there was no significant reduce in the amount of
> output with my version of gpg, as noted in the commit message you
> quoted.
> 
> > 
> > Matthew's link to
> > <https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob_plain;f=doc/DETAILS>
> > is accurate and good guidance.  As is his pointer to check the correct
> > column numbers.
> > 
> 
> Since columns may be added or removed in the format, as explained in
> this documentation, I think it'll be better not to hard-code the
> columns' numbers while parsing.

Where exactly does the gpg documentation say that columns may be *removed* in
future gpg 2.x releases?  I don't see any such statement in doc/DETAILS.

My understanding is that columns may be added *at the end* only, and never
removed until 3.x; that way, forward-compatible output parsing is possible.

That implies that our parsing should (a) hardcode column numbers per column
type, e.g., if the value of the first column is "fpr" then only check the Nth
and Mths columns, etc; and that any fields after the last *must not* be parsed
in any way.

> Therefor, I'm inclined to go through all
> of the fields after `fpr` and after `uid` in every line. With the field
> in the line with `fpr`, it's easy to be sure of the `uid` we get from
> this line since we can check if it's matched against `[A-F0-9]{40}`.
> 
> As for the line with description (or usually an email address), I'm not
> sure what regex match to test on it before putting it in the right
> array.. As already pointed out by Daniel, testing it with `=~ "@"` is
> not good enough. Perhaps here we'll use a hard-coded number as
> documented in gpg's repository? I need some opinions here..
> 

The value to match against should be obtained from a hard-coded field number
for a given value of the first line, yes.  As to matching against it by regex,
ideally we'd leave matching to compsys so matchers like _approximate could kick
in.  

> > Beware that recent versions of GnuPG always show fingerprints, for keys
> > and subkeys, because (per commit message) "The fingerprint should always
> > be used thus we should always print it."; so you'll get multiple `fpr:`
> > records per top-level key, although between the `sec` or `pub` top-level
> > introducer and the `uid:` lines for _that_ key there should just be the
> > top-level fingerprint.
> > 
> 
> 
> > 
> > Welcome to the world of GnuPG integration.  You have my sympathy.  But
> > also my encouragement.  :)
> > 
> > -Phil
> 
> Anyway, I've created a draft that should be a much better and much more
> understood but it's not ready yet as I'm not sure about what I explained
> above, this is it:
> 
> 
>     local public_keys_lines=(${(f)"$(_call_program public-keys ${(q)words[1]} ${(q)needed} --list-public-keys --list-options no-show-photos --with-colons)"})
>     local -a uids emails
>     local i j parts
>     for (( i = 1; i < ${#public_keys_lines[@]}; ++i )); do
>       parts=(${(@s.:.)public_keys_lines[$i]})
>       if [[ ${parts[1]} == "fpr" ]]; then
>         # This loop ensures that no matter if fields are added, the last field
>         # that is built from 40 upper case A-Z letters is used as the uid.

As explained above: the parsing should completely ignore any fields in an "fpr"
line beyond the 11th (since it so happens that currently 'fpr' lines have 11
fields in them).

>         # We named the variable current_uid becuase it may have many email
>         # addresses.
>         for (( j = 2; j <${#parts[@]}; ++j )); do
>           if [[ "${parts[$j]}" =~ "[A-F0-9]{40}" ]]; then
>             current_uid="${parts[$j]}"
>           fi
>         done
>         i=$((i + 1))
>         parts=(${(@s.:.)public_keys_lines[$i]})
>         while [[ ${parts[1]} == "uid" ]]; do
>           for (( j = 2; j <${#parts[@]}; ++j )); do
>             # FIXME?
>             if [[ "${parts[$j]}" =~ "@" ]]; then

Can't you just change this condition to «if true; then», so compsys would do
the matching itself>

>               uids+=("${current_uid}")
>               emails+=("${parts[$j]}")
>             fi
>           done
>           i=$((i + 1))
>           parts=(${(@s.:.)public_keys_lines[$i]})
>         done
>       fi
>     done
>     _describe -t public-keys 'public key' emails uids
> 
> For me it works great and it is much faster then before, yet I'm not
> sure about the if statement right below the `FIXME` comment.

Cheers,

Daniel