Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

openpgp: another rewrite of UID/self-sig parsing #5

Merged
merged 2 commits into from
Jan 11, 2016
Merged

Conversation

maxtaco
Copy link

@maxtaco maxtaco commented Jan 11, 2016

  • In this rewrite, we're not making a subloop for consuming signatures after reading a UID;
    we're just dropping into the loop that's already in place. We'll look for a self-sig
    so long as there's not a valid self-sig already consumed. We only take the UserID as valid
    once we find a valid self-signature. Otherwise, we'll just silently drop that UserID.
  • This should fix CORE-2344, and there's a test case for it that now works.
  • Also add another test case for Keys without any valid UIDs.

- In this rewrite, we're not making a subloop for consuming signatures after reading a UID;
  we're just dropping into the loop that's already in place. We'll look for a self-sig
  so long as there's not a valid self-sig already consumed. We only take the UserID as valid
  once we find a valid self-signature. Otherwise, we'll just silently drop that UserID.
- This should fix CORE-2344, and there's a test case for it that now works.
- Also add another test case for Keys without any valid UIDs.
@maxtaco
Copy link
Author

maxtaco commented Jan 11, 2016

@oconnor663 can you take a look at this? We should discuss it in person, since it's subtle, and I'm not 100% convinced it's correct.

@maxtaco
Copy link
Author

maxtaco commented Jan 11, 2016

(This is a fix for keybase/client#1685)

@oconnor663
Copy link

In my entirely unqualified opinion, this change looks good.

@@ -353,33 +353,22 @@ EachPacket:
current = new(Identity)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're at it, could we comment the shit out of all of these cases? :-D

maxtaco added a commit that referenced this pull request Jan 11, 2016
openpgp: another rewrite of UID/self-sig parsing
@maxtaco maxtaco merged commit b5253e1 into master Jan 11, 2016
@maxtaco maxtaco deleted the CORE-2344 branch January 11, 2016 22:06
maxtaco added a commit to keybase/client that referenced this pull request Jan 11, 2016
MarcoPolo pushed a commit to keybase/client that referenced this pull request Jan 13, 2016
@aviau
Copy link

aviau commented Jun 12, 2018

@maxtaco Do I have your permission to upstream this fix?

@maxtaco
Copy link
Author

maxtaco commented Jun 12, 2018

Yes!

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

Successfully merging this pull request may close these issues.

3 participants