Message ID | 20181213212256.48122-1-john.a.passaro@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | Expose gpgsig in pretty-print | expand |
On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote: > Currently, users who do not have GPG installed have no way to discern > signed from unsigned commits without examining raw commit data. I > propose two new pretty-print placeholders to expose this information: > > %GR: full ("R"aw) contents of gpgsig header > %G+: Y/N if the commit has nonempty gpgsig header or not > > The second is of course much more likely to be used, but having exposed > the one, exposing the other too adds almost no complexity. > > I'm open to suggestion on the names of these placeholders. > > This commit is based on master but e5a329a279 ("run-command: report exec > failure" 2018-12-11) is required for the tests to pass. > > One note is that this change touches areas of the pretty-format > documentation that are radically revamped in aw/pretty-trailers: see > 42617752d4 ("doc: group pretty-format.txt placeholders descriptions" > 2018-12-08). I have another version of this branch based on that branch > as well, so you can use that in case conflicts with aw/pretty-trailers > arise. > > See: > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers > > John Passaro (4): > pretty: expose raw commit signature > t/t7510-signed-commit.sh: test new placeholders > doc, tests: pretty behavior when gpg missing > docs/pretty-formats: add explanation + copy edits > > Documentation/pretty-formats.txt | 21 ++++-- > pretty.c | 36 ++++++++- > t/t7510-signed-commit.sh | 125 +++++++++++++++++++++++++++++-- > 3 files changed, 167 insertions(+), 15 deletions(-) > > > base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7 > prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db Just a suggestion: since the raw signature is not very useful without the commit data to check it against, and the commit data is non-trivial to construct (requires mangling raw data anyway), maybe you could either add another placeholder to get the data for signature verification, or (alternatively or simultaneously) add a placeholder that prints both data and signature in the OpenPGP message format (i.e. something you can pass straight to 'gpg --verify').
On Thu, Dec 13, 2018 at 11:12 PM Michał Górny <mgorny@gentoo.org> wrote: > > On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote: > > Currently, users who do not have GPG installed have no way to discern > > signed from unsigned commits without examining raw commit data. I > > propose two new pretty-print placeholders to expose this information: > > > > %GR: full ("R"aw) contents of gpgsig header > > %G+: Y/N if the commit has nonempty gpgsig header or not > > > > The second is of course much more likely to be used, but having exposed > > the one, exposing the other too adds almost no complexity. > > > > I'm open to suggestion on the names of these placeholders. > > > > This commit is based on master but e5a329a279 ("run-command: report exec > > failure" 2018-12-11) is required for the tests to pass. > > > > One note is that this change touches areas of the pretty-format > > documentation that are radically revamped in aw/pretty-trailers: see > > 42617752d4 ("doc: group pretty-format.txt placeholders descriptions" > > 2018-12-08). I have another version of this branch based on that branch > > as well, so you can use that in case conflicts with aw/pretty-trailers > > arise. > > > > See: > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers > > > > John Passaro (4): > > pretty: expose raw commit signature > > t/t7510-signed-commit.sh: test new placeholders > > doc, tests: pretty behavior when gpg missing > > docs/pretty-formats: add explanation + copy edits > > > > Documentation/pretty-formats.txt | 21 ++++-- > > pretty.c | 36 ++++++++- > > t/t7510-signed-commit.sh | 125 +++++++++++++++++++++++++++++-- > > 3 files changed, 167 insertions(+), 15 deletions(-) > > > > > > base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7 > > prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db > > Just a suggestion: since the raw signature is not very useful without > the commit data to check it against, and the commit data is non-trivial > to construct (requires mangling raw data anyway), maybe you could either > add another placeholder to get the data for signature verification, or > (alternatively or simultaneously) add a placeholder that prints both > data and signature in the OpenPGP message format (i.e. something you can > pass straight to 'gpg --verify'). > That's a great idea! Then I might rename the other new placeholders too: %Gs: signed commit signature (blank when unsigned) %Gp: signed commit payload (i.e. in practice minus the gpgsig header; also blank when unsigned as well) %Gq: query/question whether is signed commit ("Y"/"N") Thus establishing %G<lowercase> as the gpg-related placeholders that do not actually require gpg. And add a test that %Gp%n%Gs or the like passes gpg --verify. I'll put in a v2 later today or tomorrow. Thank you for the feedback! -- JP
On Fri, 2018-12-14 at 11:07 -0500, John Passaro wrote: > On Thu, Dec 13, 2018 at 11:12 PM Michał Górny <mgorny@gentoo.org> wrote: > > > > On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote: > > > Currently, users who do not have GPG installed have no way to discern > > > signed from unsigned commits without examining raw commit data. I > > > propose two new pretty-print placeholders to expose this information: > > > > > > %GR: full ("R"aw) contents of gpgsig header > > > %G+: Y/N if the commit has nonempty gpgsig header or not > > > > > > The second is of course much more likely to be used, but having exposed > > > the one, exposing the other too adds almost no complexity. > > > > > > I'm open to suggestion on the names of these placeholders. > > > > > > This commit is based on master but e5a329a279 ("run-command: report exec > > > failure" 2018-12-11) is required for the tests to pass. > > > > > > One note is that this change touches areas of the pretty-format > > > documentation that are radically revamped in aw/pretty-trailers: see > > > 42617752d4 ("doc: group pretty-format.txt placeholders descriptions" > > > 2018-12-08). I have another version of this branch based on that branch > > > as well, so you can use that in case conflicts with aw/pretty-trailers > > > arise. > > > > > > See: > > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig > > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers > > > > > > John Passaro (4): > > > pretty: expose raw commit signature > > > t/t7510-signed-commit.sh: test new placeholders > > > doc, tests: pretty behavior when gpg missing > > > docs/pretty-formats: add explanation + copy edits > > > > > > Documentation/pretty-formats.txt | 21 ++++-- > > > pretty.c | 36 ++++++++- > > > t/t7510-signed-commit.sh | 125 +++++++++++++++++++++++++++++-- > > > 3 files changed, 167 insertions(+), 15 deletions(-) > > > > > > > > > base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7 > > > prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db > > > > Just a suggestion: since the raw signature is not very useful without > > the commit data to check it against, and the commit data is non-trivial > > to construct (requires mangling raw data anyway), maybe you could either > > add another placeholder to get the data for signature verification, or > > (alternatively or simultaneously) add a placeholder that prints both > > data and signature in the OpenPGP message format (i.e. something you can > > pass straight to 'gpg --verify'). > > > > That's a great idea! > > Then I might rename the other new placeholders too: > > %Gs: signed commit signature (blank when unsigned) > %Gp: signed commit payload (i.e. in practice minus the gpgsig header; > also blank when unsigned as well) > %Gq: query/question whether is signed commit ("Y"/"N") > > Thus establishing %G<lowercase> as the gpg-related placeholders that > do not actually require gpg. > > And add a test that %Gp%n%Gs or the like passes gpg --verify. > > I'll put in a v2 later today or tomorrow. Thank you for the feedback! > Technically speaking, '%Gp%n%Gs' sounds a bit odd, given that the payload needs to be preceded by the PGP message header but instead of footer it has the signature's header. Also note that some lines in the payload may need to be escaped.
On Fri, Dec 14, 2018 at 11:49 AM Michał Górny <mgorny@gentoo.org> wrote: > > On Fri, 2018-12-14 at 11:07 -0500, John Passaro wrote: > > On Thu, Dec 13, 2018 at 11:12 PM Michał Górny <mgorny@gentoo.org> wrote: > > > > > > On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote: > > > > Currently, users who do not have GPG installed have no way to discern > > > > signed from unsigned commits without examining raw commit data. I > > > > propose two new pretty-print placeholders to expose this information: > > > > > > > > %GR: full ("R"aw) contents of gpgsig header > > > > %G+: Y/N if the commit has nonempty gpgsig header or not > > > > > > > > The second is of course much more likely to be used, but having exposed > > > > the one, exposing the other too adds almost no complexity. > > > > > > > > I'm open to suggestion on the names of these placeholders. > > > > > > > > This commit is based on master but e5a329a279 ("run-command: report exec > > > > failure" 2018-12-11) is required for the tests to pass. > > > > > > > > One note is that this change touches areas of the pretty-format > > > > documentation that are radically revamped in aw/pretty-trailers: see > > > > 42617752d4 ("doc: group pretty-format.txt placeholders descriptions" > > > > 2018-12-08). I have another version of this branch based on that branch > > > > as well, so you can use that in case conflicts with aw/pretty-trailers > > > > arise. > > > > > > > > See: > > > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig > > > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers > > > > > > > > John Passaro (4): > > > > pretty: expose raw commit signature > > > > t/t7510-signed-commit.sh: test new placeholders > > > > doc, tests: pretty behavior when gpg missing > > > > docs/pretty-formats: add explanation + copy edits > > > > > > > > Documentation/pretty-formats.txt | 21 ++++-- > > > > pretty.c | 36 ++++++++- > > > > t/t7510-signed-commit.sh | 125 +++++++++++++++++++++++++++++-- > > > > 3 files changed, 167 insertions(+), 15 deletions(-) > > > > > > > > > > > > base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7 > > > > prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db > > > > > > Just a suggestion: since the raw signature is not very useful without > > > the commit data to check it against, and the commit data is non-trivial > > > to construct (requires mangling raw data anyway), maybe you could either > > > add another placeholder to get the data for signature verification, or > > > (alternatively or simultaneously) add a placeholder that prints both > > > data and signature in the OpenPGP message format (i.e. something you can > > > pass straight to 'gpg --verify'). > > > > > > > That's a great idea! > > > > Then I might rename the other new placeholders too: > > > > %Gs: signed commit signature (blank when unsigned) > > %Gp: signed commit payload (i.e. in practice minus the gpgsig header; > > also blank when unsigned as well) > > %Gq: query/question whether is signed commit ("Y"/"N") > > > > Thus establishing %G<lowercase> as the gpg-related placeholders that > > do not actually require gpg. > > > > And add a test that %Gp%n%Gs or the like passes gpg --verify. > > > > I'll put in a v2 later today or tomorrow. Thank you for the feedback! > > > > Technically speaking, '%Gp%n%Gs' sounds a bit odd, given that > the payload needs to be preceded by the PGP message header but instead > of footer it has the signature's header. Also note that some lines in > the payload may need to be escaped. It's indeed failing with "-----BEGIN PGP SIGNED MESSAGE-----%n%Gp%n%Gs" This appears to be a misunderstanding on my part of how cleartext GPG messages work, as this message seems to fail verification even when I construct it manually: -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 tree 3820419b50e9827493beedf6a1423e7d9c7edf3b parent 356f37356edf1a45c8494870e1c051e2fbe529fa author A U Thor <author@example.com> 1112912413 -0700 committer C O Mitter <committer@example.com> 1112912533 -0700 sixth -----BEGIN PGP SIGNATURE----- iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCXBQzKRYcY29tbWl0dGVy QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN2QYAnA2A/VpD4qMI9rJlIYnyyO32rTXz AJ0drWlJsASMcf6AEX6nSQPxWq81Fg== =fr2F -----END PGP SIGNATURE----- I got this by running tho following inside the trash directory for t7510, which as far as I can tell is roughly equivalent to The gpg --verify fails. { echo "-----BEGIN PGP SIGNED MESSAGE-----" echo Hash: SHA256 echo git cat-file commit sixth-signed | perl -ne '/^(?:gpgsig)? / || print' git cat-file commit sixth-signed | perl -ne 's/^(?:gpgsig)? // && print' } | tee commit-as-signed-message | gpg --verify All seems to work fine when I treat %Gs as a detached signature. How should the combined message be constructed properly? (Goes to usefulness of printing signature payload, and indeed of raw crypto data in general.) > -- > Best regards, > Michał Górny
On Fri, Dec 14, 2018 at 6:10 PM John Passaro <john.a.passaro@gmail.com> wrote: > > On Fri, Dec 14, 2018 at 11:49 AM Michał Górny <mgorny@gentoo.org> wrote: > > > > On Fri, 2018-12-14 at 11:07 -0500, John Passaro wrote: > > > On Thu, Dec 13, 2018 at 11:12 PM Michał Górny <mgorny@gentoo.org> wrote: > > > > > > > > On Thu, 2018-12-13 at 16:22 -0500, John Passaro wrote: > > > > > Currently, users who do not have GPG installed have no way to discern > > > > > signed from unsigned commits without examining raw commit data. I > > > > > propose two new pretty-print placeholders to expose this information: > > > > > > > > > > %GR: full ("R"aw) contents of gpgsig header > > > > > %G+: Y/N if the commit has nonempty gpgsig header or not > > > > > > > > > > The second is of course much more likely to be used, but having exposed > > > > > the one, exposing the other too adds almost no complexity. > > > > > > > > > > I'm open to suggestion on the names of these placeholders. > > > > > > > > > > This commit is based on master but e5a329a279 ("run-command: report exec > > > > > failure" 2018-12-11) is required for the tests to pass. > > > > > > > > > > One note is that this change touches areas of the pretty-format > > > > > documentation that are radically revamped in aw/pretty-trailers: see > > > > > 42617752d4 ("doc: group pretty-format.txt placeholders descriptions" > > > > > 2018-12-08). I have another version of this branch based on that branch > > > > > as well, so you can use that in case conflicts with aw/pretty-trailers > > > > > arise. > > > > > > > > > > See: > > > > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig > > > > > - https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers > > > > > > > > > > John Passaro (4): > > > > > pretty: expose raw commit signature > > > > > t/t7510-signed-commit.sh: test new placeholders > > > > > doc, tests: pretty behavior when gpg missing > > > > > docs/pretty-formats: add explanation + copy edits > > > > > > > > > > Documentation/pretty-formats.txt | 21 ++++-- > > > > > pretty.c | 36 ++++++++- > > > > > t/t7510-signed-commit.sh | 125 +++++++++++++++++++++++++++++-- > > > > > 3 files changed, 167 insertions(+), 15 deletions(-) > > > > > > > > > > > > > > > base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7 > > > > > prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db > > > > > > > > Just a suggestion: since the raw signature is not very useful without > > > > the commit data to check it against, and the commit data is non-trivial > > > > to construct (requires mangling raw data anyway), maybe you could either > > > > add another placeholder to get the data for signature verification, or > > > > (alternatively or simultaneously) add a placeholder that prints both > > > > data and signature in the OpenPGP message format (i.e. something you can > > > > pass straight to 'gpg --verify'). > > > > > > > > > > That's a great idea! > > > > > > Then I might rename the other new placeholders too: > > > > > > %Gs: signed commit signature (blank when unsigned) > > > %Gp: signed commit payload (i.e. in practice minus the gpgsig header; > > > also blank when unsigned as well) > > > %Gq: query/question whether is signed commit ("Y"/"N") > > > > > > Thus establishing %G<lowercase> as the gpg-related placeholders that > > > do not actually require gpg. > > > > > > And add a test that %Gp%n%Gs or the like passes gpg --verify. > > > > > > I'll put in a v2 later today or tomorrow. Thank you for the feedback! > > > > > > > Technically speaking, '%Gp%n%Gs' sounds a bit odd, given that > > the payload needs to be preceded by the PGP message header but instead > > of footer it has the signature's header. Also note that some lines in > > the payload may need to be escaped. > > It's indeed failing with > "-----BEGIN PGP SIGNED MESSAGE-----%n%Gp%n%Gs" > > This appears to be a misunderstanding on my part of how cleartext GPG > messages work, as this message seems to fail verification even when I > construct it manually: > > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > tree 3820419b50e9827493beedf6a1423e7d9c7edf3b > parent 356f37356edf1a45c8494870e1c051e2fbe529fa > author A U Thor <author@example.com> 1112912413 -0700 > committer C O Mitter <committer@example.com> 1112912533 -0700 > > sixth > -----BEGIN PGP SIGNATURE----- > > iHQEABECADQWIQRz11h0S+chaY7FTocTtvUezd5DDQUCXBQzKRYcY29tbWl0dGVy > QGV4YW1wbGUuY29tAAoJEBO29R7N3kMN2QYAnA2A/VpD4qMI9rJlIYnyyO32rTXz > AJ0drWlJsASMcf6AEX6nSQPxWq81Fg== > =fr2F > -----END PGP SIGNATURE----- > > I got this by running tho following inside the trash directory for t7510, > which as far as I can tell is roughly equivalent to > The gpg --verify fails. > { > echo "-----BEGIN PGP SIGNED MESSAGE-----" > echo Hash: SHA256 > echo > git cat-file commit sixth-signed | perl -ne '/^(?:gpgsig)? / || print' > git cat-file commit sixth-signed | perl -ne 's/^(?:gpgsig)? // && print' > } | tee commit-as-signed-message | gpg --verify I should add that when I took out Hash: SHA256 (don't ask), gpg went from saying "signature digest conflict" to saying "BAD signature". Not quite as bad but still confusing. > > All seems to work fine when I treat %Gs as a detached signature. > How should the combined message be constructed properly? (Goes > to usefulness of printing signature payload, and indeed of raw crypto > data in general.) > > > > -- > > Best regards, > > Michał Górny
Michał Górny <mgorny@gentoo.org> writes: > Just a suggestion: since the raw signature is not very useful without > the commit data to check it against, and the commit data is non-trivial > to construct (requires mangling raw data anyway), maybe you could either > add another placeholder to get the data for signature verification, or > (alternatively or simultaneously) add a placeholder that prints both > data and signature in the OpenPGP message format (i.e. something you can > pass straight to 'gpg --verify'). Yeah, the last would be the most usable; anything short of that, I have to suspect that going from "cat-file commit", rather than using this new %Gsomething placeholder, would be more practical.
On Fri, Dec 14, 2018 at 11:07:03AM -0500, John Passaro wrote: > Then I might rename the other new placeholders too: > > %Gs: signed commit signature (blank when unsigned) > %Gp: signed commit payload (i.e. in practice minus the gpgsig header; > also blank when unsigned as well) One complication: the pretty-printing code sees the commit data in the i18n.logOutputEncoding charset (utf8 by default). But the signature will be over the raw commit data. That's also utf8 by default, but there may be an encoding header indicating that it's something else. In that case, you couldn't actually verify the signature from the "%Gs%Gp" pair. I don't think that's insurmountable in the code. You'll have to jump through a few hoops to make sure you have the _original_ payload, but we obviously do have that data. However, it does feel a little weird to include content from a different encoding in the middle of the log output stream which claims to be i18n.logOutputEncoding. -Peff
On Fri, Dec 14, 2018 at 6:10 PM John Passaro wrote: > All seems to work fine when I treat %Gs as a detached signature. In light of this, my best guess as to why the cleartext PGP message didn't verify properly is that the commit data normally doesn't end with \n, but as far as I can tell there's no way to express that in the cleartext format. I don't see a way around this. However, as long as the following works, I think we have proof-of-concept that this enhancement allows you to play with signature data however you please without leaving it to git under the hood: gpg --verify <(git show -s --format=format:%Gs commit) <(git show -s --format=format:%Gp commit) On Mon, Dec 17, 2018 at 3:24 PM Jeff King <peff@peff.net> wrote: > > On Fri, Dec 14, 2018 at 11:07:03AM -0500, John Passaro wrote: > > > Then I might rename the other new placeholders too: > > > > %Gs: signed commit signature (blank when unsigned) > > %Gp: signed commit payload (i.e. in practice minus the gpgsig header; > > also blank when unsigned as well) > > One complication: the pretty-printing code sees the commit data in the > i18n.logOutputEncoding charset (utf8 by default). But the signature will > be over the raw commit data. That's also utf8 by default, but there may > be an encoding header indicating that it's something else. In that case, > you couldn't actually verify the signature from the "%Gs%Gp" pair. > > I don't think that's insurmountable in the code. You'll have to jump > through a few hoops to make sure you have the _original_ payload, but we > obviously do have that data. However, it does feel a little weird to > include content from a different encoding in the middle of the log > output stream which claims to be i18n.logOutputEncoding. > Thanks for the feedback! This is an interesting conflict. If the user requests %Gp, the payload for the signature, they almost certainly do want it in the original encoding; if i18n.logOutputEncoding is something incompatible, whether explicitly or by default, that seems like an error. Not much we can do to reconcile the two requests (commit encoding vs output encoding) so seems reasonable to treat it as fatal. Updated patch coming as soon as I work out Peff's aforementioned "few hoops" to get properly encoded data -- and also how to test success and failure!
On Wed, 2018-12-19 at 00:59 -0500, John Passaro wrote: > On Fri, Dec 14, 2018 at 6:10 PM John Passaro wrote: > > All seems to work fine when I treat %Gs as a detached signature. > > In light of this, my best guess as to why the cleartext PGP message > didn't verify properly is that the commit data normally doesn't end > with \n, but as far as I can tell there's no way to express that in > the cleartext format. I don't see a way around this. You are most likely right. I've just skimmed through RFC 4880 and indeed it seems to rely on the newline encoding being quite normalized in the message. > However, as long > as the following works, I think we have proof-of-concept that this > enhancement allows you to play with signature data however you please > without leaving it to git under the hood: > > gpg --verify <(git show -s --format=format:%Gs commit) <(git show -s > --format=format:%Gp commit) That's a nice trick. Thanks for the effort you're putting into this! > > On Mon, Dec 17, 2018 at 3:24 PM Jeff King <peff@peff.net> wrote: > > > > On Fri, Dec 14, 2018 at 11:07:03AM -0500, John Passaro wrote: > > > > > Then I might rename the other new placeholders too: > > > > > > %Gs: signed commit signature (blank when unsigned) > > > %Gp: signed commit payload (i.e. in practice minus the gpgsig header; > > > also blank when unsigned as well) > > > > One complication: the pretty-printing code sees the commit data in the > > i18n.logOutputEncoding charset (utf8 by default). But the signature will > > be over the raw commit data. That's also utf8 by default, but there may > > be an encoding header indicating that it's something else. In that case, > > you couldn't actually verify the signature from the "%Gs%Gp" pair. > > > > I don't think that's insurmountable in the code. You'll have to jump > > through a few hoops to make sure you have the _original_ payload, but we > > obviously do have that data. However, it does feel a little weird to > > include content from a different encoding in the middle of the log > > output stream which claims to be i18n.logOutputEncoding. > > > > Thanks for the feedback! This is an interesting conflict. If the user > requests %Gp, the payload for the signature, they almost certainly do > want it in the original encoding; if i18n.logOutputEncoding is > something incompatible, whether explicitly or by default, that seems > like an error. Not much we can do to reconcile the two requests > (commit encoding vs output encoding) so seems reasonable to treat it > as fatal. > > Updated patch coming as soon as I work out Peff's aforementioned "few > hoops" to get properly encoded data -- and also how to test success > and failure!