~sircmpwn/aerc

6 3

[WIP] Remove hard coded path everywhere

Details
Message ID
<20200619160653.255355-1-reto@labrat.space>
DKIM signature
pass
Download raw message
Patch: +45 -13
---
This gets rid of the hardcoded body part path everywhere.

Some things need to be fixed though, at least the following:

* duplicated findPlaintext, findFirstNonMultipart functions, need to be
  moved to a separate package so that we can use it in lib and command
* still fails for some emails, ironically for the gpg test email from sr.ht for
  example

 commands/msg/forward.go  |  4 +---
 commands/msg/recall.go   |  1 -
 commands/msg/reply.go    |  2 +-
 widgets/msgviewer.go     |  2 +-
 worker/lib/parse.go      | 10 ++++------
 worker/maildir/search.go | 39 ++++++++++++++++++++++++++++++++++++++-
 6 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/commands/msg/forward.go b/commands/msg/forward.go
index 28abbed..1436960 100644
--- a/commands/msg/forward.go
+++ b/commands/msg/forward.go
@@ -140,9 +140,7 @@ func (forward) Execute(aerc *widgets.Aerc, args []string) error {
		part := findPlaintext(msg.BodyStructure, nil)
		if part == nil {
			part = findFirstNonMultipart(msg.BodyStructure, nil)
			if part == nil {
				part = []int{1}
			}
			// if it's still nil here, we don't have a multipart msg, that's fine
		}
		store.FetchBodyPart(msg.Uid, part, func(reader io.Reader) {
			buf := new(bytes.Buffer)
diff --git a/commands/msg/recall.go b/commands/msg/recall.go
index 7c9ac19..4f38a77 100644
--- a/commands/msg/recall.go
+++ b/commands/msg/recall.go
@@ -112,7 +112,6 @@ func (Recall) Execute(aerc *widgets.Aerc, args []string) error {
	part, err = msgInfo.BodyStructure.PartAtIndex(path)
	if part == nil || err != nil {
		part = msgInfo.BodyStructure
		path = []int{1}
	}

	store.FetchBodyPart(msgInfo.Uid, path, func(reader io.Reader) {
diff --git a/commands/msg/reply.go b/commands/msg/reply.go
index 28ce245..d268bef 100644
--- a/commands/msg/reply.go
+++ b/commands/msg/reply.go
@@ -175,7 +175,7 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
			part = findFirstNonMultipart(msg.BodyStructure, nil)
			if part == nil {
				// give up, use whatever is first
				part = []int{1}
				part = nil
			}
		}
		store.FetchBodyPart(msg.Uid, part, func(reader io.Reader) {
diff --git a/widgets/msgviewer.go b/widgets/msgviewer.go
index f06b787..a7b9fd6 100644
--- a/widgets/msgviewer.go
+++ b/widgets/msgviewer.go
@@ -180,7 +180,7 @@ func createSwitcher(acct *AccountView, switcher *PartSwitcher,

	if len(msg.BodyStructure().Parts) == 0 {
		switcher.selected = 0
		pv, err := NewPartViewer(acct, conf, msg, msg.BodyStructure(), []int{1})
		pv, err := NewPartViewer(acct, conf, msg, msg.BodyStructure(), nil)
		if err != nil {
			return err
		}
diff --git a/worker/lib/parse.go b/worker/lib/parse.go
index bbea49d..95db6e6 100644
--- a/worker/lib/parse.go
+++ b/worker/lib/parse.go
@@ -21,8 +21,9 @@ var dateRe = regexp.MustCompile(`(((Mon|Tue|Wed|Thu|Fri|Sat|Sun))[,]?\s[0-9]{1,2
	`([0-9]{4})\s([0-9]{2}):([0-9]{2})(:([0-9]{2}))?\s([\+|\-][0-9]{4})\s?`)

func FetchEntityPartReader(e *message.Entity, index []int) (io.Reader, error) {
	if len(index) < 1 {
		return nil, fmt.Errorf("no part to read")
	if len(index) == 0 {
		// non multipart, simply return everything
		return bufReader(e)
	}
	if mpr := e.MultipartReader(); mpr != nil {
		idx := 0
@@ -41,10 +42,7 @@ func FetchEntityPartReader(e *message.Entity, index []int) (io.Reader, error) {
			}
		}
	}
	if index[0] != 1 {
		return nil, fmt.Errorf("cannont return non-first part of non-multipart")
	}
	return bufReader(e)
	return nil, fmt.Errorf("FetchEntityPartReader: unexpected code reached")
}

//TODO: the UI doesn't seem to like readers which aren't buffers
diff --git a/worker/maildir/search.go b/worker/maildir/search.go
index f658c02..25d04ff 100644
--- a/worker/maildir/search.go
+++ b/worker/maildir/search.go
@@ -121,7 +121,12 @@ func (w *Worker) searchKey(key uint32, criteria *searchCriteria,
	}
	if parts&BODY > 0 {
		// TODO: select which part to search, maybe look for text/plain
		reader, err := message.NewBodyPartReader([]int{1})
		mi, err := message.MessageInfo()
		if err != nil {
			return false, err
		}
		path := findFirstNonMultipart(mi.BodyStructure, nil)
		reader, err := message.NewBodyPartReader(path)
		if err != nil {
			return false, err
		}
@@ -248,3 +253,35 @@ func getRequiredParts(criteria *searchCriteria) MsgParts {

	return required
}

func findFirstNonMultipart(bs *models.BodyStructure, path []int) []int {
	for i, part := range bs.Parts {
		cur := append(path, i+1)
		mimetype := strings.ToLower(part.MIMEType)
		if mimetype != "multipart" {
			return path
		} else if mimetype == "multipart" {
			if path := findPlaintext(part, cur); path != nil {
				return path
			}
		}
	}
	return nil
}

func findPlaintext(bs *models.BodyStructure, path []int) []int {
	for i, part := range bs.Parts {
		cur := append(path, i+1)
		if strings.ToLower(part.MIMEType) == "text" &&
			strings.ToLower(part.MIMESubType) == "plain" {
			return cur
		}
		if strings.ToLower(part.MIMEType) == "multipart" {
			if path := findPlaintext(part, cur); path != nil {
				return path
			}
		}
	}
	return nil
}

-- 
2.27.0
Details
Message ID
<KHJqUbVFZOFwPFId1d-RTnR2Zoea4IIrACqGfm1Y2dvpY8WTjJXG7W438hrM3X9sm-R2wswnx9EcngrkkvuirmkY9yV1PFQ2b6AqBCYucIk=@protonmail.ch>
In-Reply-To
<20200619160653.255355-1-reto@labrat.space> (view parent)
DKIM signature
pass
Download raw message
Hi Reto,

Thanks for sending this over, am beginning to get a much better
understanding of how aerc functions.

> * still fails for some emails, ironically for the gpg test email from
> sr.ht for example

I sent myself the test pgp email but was able to open this fine. Do you
mean that some of the email is not being displayed?

I was also curious as to whether you knew of a tool to send raw email,
with the ability to add new MIME message parts, for testing purposes.

Thanks,
Michael
Details
Message ID
<_pYb4dKOmCDziC7d0jHqWkXVVzenAW6KOv_LB_mTfsDkSTaRJNHIwVyeMl1eL58xTKadVv5rgOtWXJDEfU0Okv10VQ5nUYOAchaS-0yg8CM=@protonmail.ch>
In-Reply-To
<20200619160653.255355-1-reto@labrat.space> (view parent)
DKIM signature
pass
Download raw message
Hi Reto,

I have been looking into why the test pgp email does not appear properly
and it seems it is because go-pgpmail does not look for more pgp
encrypted body parts inside a pgp encrypted message. I am working on a
fix for this, but had a question:

What is the desired behaviour of aerc when an email has a pgp signed/encrypted
body part inside a pgp encrypted body part? (As is the case with the
test email).

If the inner message has been encrypted with a different public key than
the outer message, which key should be displayed in the encryption
status of aerc?

Apologies for the delay, have been moving house.

Thanks,
Michael
Details
Message ID
<20200627103537.6uhrgl5ta6owzme3@feather.localdomain>
In-Reply-To
<KHJqUbVFZOFwPFId1d-RTnR2Zoea4IIrACqGfm1Y2dvpY8WTjJXG7W438hrM3X9sm-R2wswnx9EcngrkkvuirmkY9yV1PFQ2b6AqBCYucIk=@protonmail.ch> (view parent)
DKIM signature
pass
Download raw message
On Sat, Jun 20, 2020 at 04:57:29PM +0000, Michael Clancy wrote:
> I sent myself the test pgp email but was able to open this fine. Do you
> mean that some of the email is not being displayed?

Ehm, well, I've dumped an email I've received from a bug report into aerc
and it doesn't display anything when I open it... maybe a fluke then if it works
with the current emails.

> I was also curious as to whether you knew of a tool to send raw email,
> with the ability to add new MIME message parts, for testing purposes.

cat and msmtp in my case.
as for mime, you can write it by hand if you don't want a fancy encoding and otherwise
use something like emersion's go-message library to generate it.
Details
Message ID
<20200627104156.piq65yrrvlzidcam@feather.localdomain>
In-Reply-To
<_pYb4dKOmCDziC7d0jHqWkXVVzenAW6KOv_LB_mTfsDkSTaRJNHIwVyeMl1eL58xTKadVv5rgOtWXJDEfU0Okv10VQ5nUYOAchaS-0yg8CM=@protonmail.ch> (view parent)
DKIM signature
pass
Download raw message
Hi Michael,

On Sat, Jun 27, 2020 at 08:38:15AM +0000, Michael Clancy wrote:
> What is the desired behaviour of aerc when an email has a pgp signed/encrypted
> body part inside a pgp encrypted body part? (As is the case with the
> test email).

What do you mean exactly?
Aerc doesn't support nested emails yet.
In my opinion we should strip and validate exactly one signature from the message.
If a part is then further encrypted, that should be the task of a filter of some sort.
At least until we actually would support nested emails (not sure how that would look
from a UX side of things...)

But who on earth does nested signing and why?

> If the inner message has been encrypted with a different public key than
> the outer message, which key should be displayed in the encryption
> status of aerc?
The outermost one, that's the one you are actually supposed to verify after all.
Else you open yourself up to some nasty attacks, where some invalid email is generated
but has an attached valid signed message and suddenly aerc lights up green where
it should start screaming that the email is forged.

> Apologies for the delay, have been moving house.
No worries, real life takes priority.
Thanks for following up.

Btw: Drew might have more opinions here than I, he actually wrote the gpg code.

Cheers,
Reto
Details
Message ID
<5eBM8YE9EhJxBv-sKVqZsKW5xpW7z4RKDUTFK_F-ilbdKu2E7b7ddxZtz4pM7p5iW2Kmf2ylPYwDr5ItjdlAErRw9rqnxIdDohFIAlQvsTI=@protonmail.ch>
In-Reply-To
<20200627104156.piq65yrrvlzidcam@feather.localdomain> (view parent)
DKIM signature
pass
Download raw message
On Sat Jun 27, 2020 at 11:41 AM BST, Reto wrote:
> > What is the desired behaviour of aerc when an email has a pgp signed/encrypted
> > body part inside a pgp encrypted body part? (As is the case with the
> > test email).
>
> What do you mean exactly?
> Aerc doesn't support nested emails yet.
> In my opinion we should strip and validate exactly one signature from
> the message.
> If a part is then further encrypted, that should be the task of a filter
> of some sort.

What I mean is that some emails may have a pgp-signature body part
inside a pgp-encrypted message, as is the case for the sr.ht pgp test
email.

Currently aerc will not check inside a pgp message once decrypted for
any more body parts. The UI will then display a green encrypted, but has
not checked the signature. I think this should probably be checked for.

Thanks
Details
Message ID
<2ynfh0I_w7sJhZ-ocovw3f7ZophTl3KOmNDPmSQT_A1ySSbhxVUTX4-fkVQ_XfeVVJpmRLwbJgcB27ux3SA7ndkKPehWo_G1i6NFHUm1hr0=@emersion.fr>
In-Reply-To
<5eBM8YE9EhJxBv-sKVqZsKW5xpW7z4RKDUTFK_F-ilbdKu2E7b7ddxZtz4pM7p5iW2Kmf2ylPYwDr5ItjdlAErRw9rqnxIdDohFIAlQvsTI=@protonmail.ch> (view parent)
DKIM signature
pass
Download raw message
On Saturday, June 27, 2020 1:17 PM, Michael Clancy <mjclancy@protonmail.ch> wrote:

> On Sat Jun 27, 2020 at 11:41 AM BST, Reto wrote:
>
> > > What is the desired behaviour of aerc when an email has a pgp signed/encrypted
> > > body part inside a pgp encrypted body part? (As is the case with the
> > > test email).
> >
> > What do you mean exactly?
> > Aerc doesn't support nested emails yet.
> > In my opinion we should strip and validate exactly one signature from
> > the message.
> > If a part is then further encrypted, that should be the task of a filter
> > of some sort.
>
> What I mean is that some emails may have a pgp-signature body part
> inside a pgp-encrypted message, as is the case for the sr.ht pgp test
> email.

go-pgpmail should take care of these. It should support these already,
if it doesn't it's a bug.
Export thread (mbox)