This is a hack I needed to do to get aerc to open up my email folder,
otherwise it would display [..] and I could not select or open
email. Rather than using less/mv to read my maildir, I thought I'd
make a small change to make aerc a little more tolerant.
I'm sort of creating this to have a discussion rather than to say this
would be the right way to actually fix it. It looked like the parsing
code was in another library and I didn't review it.
The email at fault had a CC line like:
"Cc: , emacs-devel@gnu.org"
---
worker/lib/parse.go | 29 ++++++++---------------------
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/worker/lib/parse.go b/worker/lib/parse.go
index 288dade..ff5bace 100644
--- a/worker/lib/parse.go+++ b/worker/lib/parse.go
@@ -101,26 +101,11 @@ func parseEnvelope(h *mail.Header) (*models.Envelope, error) {
if err != nil {
return nil, fmt.Errorf("could not parse date header: %v", err)
}
- from, err := parseAddressList(h, "from")- if err != nil {- return nil, fmt.Errorf("could not read from address: %v", err)- }- to, err := parseAddressList(h, "to")- if err != nil {- return nil, fmt.Errorf("could not read to address: %v", err)- }- cc, err := parseAddressList(h, "cc")- if err != nil {- return nil, fmt.Errorf("could not read cc address: %v", err)- }- bcc, err := parseAddressList(h, "bcc")- if err != nil {- return nil, fmt.Errorf("could not read bcc address: %v", err)- }- replyTo, err := parseAddressList(h, "reply-to")- if err != nil {- return nil, fmt.Errorf("could not read reply-to address: %v", err)- }+ from, _ := parseAddressList(h, "from")+ to, _ := parseAddressList(h, "to")+ cc, _ := parseAddressList(h, "cc")+ bcc, _ := parseAddressList(h, "bcc")+ replyTo, _ := parseAddressList(h, "reply-to") subj, err := h.Subject()
if err != nil {
return nil, fmt.Errorf("could not read subject: %v", err)
@@ -150,7 +135,9 @@ func parseAddressList(h *mail.Header, key string) ([]*models.Address, error) {
Name: hdr,
}}, nil
}
- return nil, err+ return []*models.Address{&models.Address{+ Name: err.Error(),+ }}, err }
for _, addr := range addrs {
parts := strings.Split(addr.Address, "@")
--
2.20.1
I think this is already supposed to be handled by parseAddressList.
The condition is inverted on line 148:
if hdr, err := h.Text(key); err != nil && strings.Contains(hdr, "@") {
Should read like this:
if hdr, err := h.Text(key); err == nil && strings.Contains(hdr, "@") {
On Thu Jan 30, 2020 at 08:53, Simon Ser wrote:
> I think this is already supposed to be handled by parseAddressList.
That is correct.
> if hdr, err := h.Text(key); err == nil && strings.Contains(hdr, "@") {
The strings.Contains also was inverted in
baa70469c3b25e1b937c9c2a9e0b16762a227bca, it should be
!strings.Contains. Which won't actually solve the problem for the
message you're seeing; perhaps we can drop the strings.Contains
altogether.
In any case, parseAddressList is meant to be lenient.
On Thu Jan 30, 2020 at 11:19 AM, Ben Burwell wrote:
> The strings.Contains also was inverted in> baa70469c3b25e1b937c9c2a9e0b16762a227bca, it should be> !strings.Contains. Which won't actually solve the problem for the> message you're seeing; perhaps we can drop the strings.Contains> altogether.>> In any case, parseAddressList is meant to be lenient.
I don't remember string.Contains being an issue when I looked at
it... I think it's go's net/mail/message.go's parseAddressList(). It
expects an email address before the first comma, so it starts parsing
the comma as an email address, gives up, and returns an error. (I'm
not 100% sure about this, but just what I gathered from a quick
glance)
I don't think anyone would change go's parseAddressList to handle
this, but I would expect aerc to be able to show something...so that I
could delete it or pipe it to an editor if I want it to be reparsed
before replying, etc.
I was talking about aerc's parseAddressList function in
worker/lib/parse.go, which calls Header.AddressList from emersion's
go-message library, which in turn calls AddressParser.ParseList from the
standard library.
Both the standard library and emersion's go-message are pretty strict in
their adherence to the RFC 5322. Aerc's parseAddressList function is
meant to attempt parsing with go-message, but if an unparsable address
list is found for whatever reason, it should still return something
useful for the UI.
So I think the solution is 2-fold:
1) update aerc's parseAddressList to fix the err == nil check and remove
strings.Contains condition, and
2) see if go would accept an upstream patch to its parseAddressList
function which ignores an empty address.
On Fri Jan 31, 2020 at 10:13 AM, Ben Burwell wrote:
> So I think the solution is 2-fold:>> 1) update aerc's parseAddressList to fix the err == nil check and remove> strings.Contains condition, and
Created a v2 patch for this.
> 2) see if go would accept an upstream patch to its parseAddressList> function which ignores an empty address.https://github.com/golang/go/pull/36966