Skip to content

Fix gpg output parsing #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

guillemj
Copy link

Use «foreach» instead of «while» to traverse the GnuPG output array, otherwise the topic variable never gets initialized, and the «while» continuously evaluates the array in boolean context and we get into infinite loops.

With GnuPG the infinite loop currently never triggers because it does not output anything on stdout. But with the Sequoia GnuPG Chameleon it outputs the original contents being verified (which is a divergence that should probably be fixed upstream).

Force the output to stdout instead of stderr so that we can parse it, and update the parser to match on current output lines. Although ideally the parser should be switched to try to use one of the machine parseable outputs such as --with-colons, otherwise there is no guarantee this will not change again in the future, but this is the simplest minimal change.

Use «foreach» instead of «while» to traverse the GnuPG output array,
otherwise the topic variable never gets initialized, and the «while»
continuously evaluates the array in boolean context and we get into
infinite loops.

With GnuPG the infinite loop currently never triggers because it does
not output anything on stdout. But with the Sequoia GnuPG Chameleon it
outputs the original contents being verified (which is a divergence
that should probably be fixed upstream).

Force the output to stdout instead of stderr so that we can parse it,
and update the parser to match on current output lines. Although ideally
the parser should be switched to try to use one of the machine parseable
outputs such as --with-colons, otherwise there is no guarantee this will
not change again in the future, but this is the simplest minimal change.
@timlegge
Copy link
Collaborator

I will review in a couple of weeks. Unfortunately I am about to be off line for a bit.

@timlegge timlegge self-assigned this Mar 13, 2025
@timlegge
Copy link
Collaborator

Also, it would be nice if there was a test addition that demonstrates the issue and the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants