Skip to content
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

git am cannot parse the mbox when PGP signatures are present #221

Open
jgunthorpe opened this issue Oct 25, 2018 · 5 comments
Open

git am cannot parse the mbox when PGP signatures are present #221

jgunthorpe opened this issue Oct 25, 2018 · 5 comments
Labels

Comments

@jgunthorpe
Copy link

Here is an example from the kernel mailing list:

https://patchwork.kernel.org/patch/10656215/mbox/

The original mbox is attached to this issue.

Patchwork removed all the MIME encoding and signature but retained the Content-Type header saying it is in multipart/signed format.

'git am' knows how to parse this header and gets confused when the mailbox is no longer in the right format.

patchworks should also drop the content-type header if it is going to do this kind of reformatting.

orig.mbox.gz

@stephenfin
Copy link
Member

@jgunthorpe This is likely introduced by 01b9cbb. Previously, this header would not have been copied in as we restricted the possible headers one could use. Is there any particular reason to include the signature in the downloaded mbox? If not, I'm inclined to think the correct behavior here would be to special case Content-Type: multipart/signed (or Content-Type in general) and strip this out.

@stephenfin stephenfin added the bug label Oct 26, 2018
@jgunthorpe
Copy link
Author

I don't think git am can currently validate the gpg signature so I don't think there is any reason to provide it at this time. Stripping/rewriting content-type, content-length and content-encoding is probably the right thing to do if the body is being rewritten.

stephenfin added a commit that referenced this issue Dec 22, 2018
We don't GPG signatures, therefore this header is incorrect. Stop
passing it through.

Test for the other dropped header are also included.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Acked-by: Veronika Kabatova <vkabatov@redhat.com>
Closes: #221
(cherry picked from commit 2209369)
daxtens pushed a commit to daxtens/patchwork that referenced this issue Apr 30, 2019
We don't GPG signatures, therefore this header is incorrect. Stop
passing it through.

Test for the other dropped header are also included.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Acked-by: Veronika Kabatova <vkabatov@redhat.com>
Closes: getpatchwork#221
(cherry picked from commit 2209369)
Signed-off-by: Daniel Axtens <dja@axtens.net>
@andy-shev
Copy link

andy-shev commented May 6, 2019

Should be reopened since this broke non-manual application of the patch. If you don't supply a signature the Content type of the mbox has to be mangled accordingly.

Patch to test with: https://patchwork.kernel.org/patch/10921937/

@jgunthorpe
Copy link
Author

I think this is the same breakage reported here.. It kind of looks like the patch may not understand the extended Content-type header being used here:

Content-Type: multipart/signed; micalg=pgp-sha1;
        protocol="application/pgp-signature"; boundary="EeQfGwPcQSOJBaQU"

@andy-shev
Copy link

andy-shev commented May 6, 2019

Actually git am the tool which fails. patch works nicely on this. I tried to download a mailbox via git pw and pwclient with the same result. I think either both tools are broken, or patchwork itself.

@stephenfin stephenfin reopened this May 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants