Thread: archive not showing all attachements

Hi,

In a recent email Horiguchi-san attached an incremental patch as .txt,
presumably to avoid confusing cfbot. I just noticed that the archives don't
offer a download link:
https://www.postgresql.org/message-id/20220323.163833.1929400812667041796.horikyota.ntt%40gmail.com

but in my mail reader it's perfectly visible.

I don't know if it's caused by the file ending or Horiguchi-san's email client
using text/Plain rather than text/plain. As far as I understand mime types are
supposed to be case insensitive, so text/Plain is perfectly correct, albeit a
little unusual.

Greetings,

Andres Freund

On 3/24/22 3:24 PM, Andres Freund wrote:
> Hi,
> 
> In a recent email Horiguchi-san attached an incremental patch as .txt,
> presumably to avoid confusing cfbot. I just noticed that the archives don't
> offer a download link:
> https://www.postgresql.org/message-id/20220323.163833.1929400812667041796.horikyota.ntt%40gmail.com
> 
> but in my mail reader it's perfectly visible.
> 
> I don't know if it's caused by the file ending or Horiguchi-san's email client
> using text/Plain rather than text/plain. As far as I understand mime types are
> supposed to be case insensitive, so text/Plain is perfectly correct, albeit a
> little unusual.

I ran into a similar issue while using Mac mail, where attached patches 
wouldn't post to the archives. I switched to Thunderbird solely for 
posting to the mailing lists.

That said, that may be a thread worth pulling. It looks like we do 
case-sensitivity checking on the MIME type based on the 
"get_content_type" docs[1][2].

If you have the raw message, it may be good to manually run through the 
code and see what's happening in the the "recursive_get_attachments" to 
see root cause and potentially fix this.

(I don't have access to the server side directly, so I can't test it. I 
may be able to parse it from the mbox, which in that case, I'll try this 
out later).

Jonathan

[1] 
https://git.postgresql.org/gitweb/?p=pgarchives.git;a=blob;f=loader/lib/parser.py;hb=HEAD#l257
[2] 
https://docs.python.org/3/library/email.message.html#email.message.EmailMessage.get_content_type

On Thu, Mar 24, 2022 at 9:18 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
>
> On 3/24/22 3:24 PM, Andres Freund wrote:
> > Hi,
> >
> > In a recent email Horiguchi-san attached an incremental patch as .txt,
> > presumably to avoid confusing cfbot. I just noticed that the archives don't
> > offer a download link:
> > https://www.postgresql.org/message-id/20220323.163833.1929400812667041796.horikyota.ntt%40gmail.com
> >
> > but in my mail reader it's perfectly visible.
> >
> > I don't know if it's caused by the file ending or Horiguchi-san's email client
> > using text/Plain rather than text/plain. As far as I understand mime types are
> > supposed to be case insensitive, so text/Plain is perfectly correct, albeit a
> > little unusual.
>
> I ran into a similar issue while using Mac mail, where attached patches
> wouldn't post to the archives. I switched to Thunderbird solely for
> posting to the mailing lists.
>
> That said, that may be a thread worth pulling. It looks like we do
> case-sensitivity checking on the MIME type based on the
> "get_content_type" docs[1][2].
>
> If you have the raw message, it may be good to manually run through the
> code and see what's happening in the the "recursive_get_attachments" to
> see root cause and potentially fix this.

There is a link to the raw message directly from that page in the
archives, remember? :)

That said, it's not as simple as a upper/lowercase. get_content_type()
returns 'text/plain' in lowercase for both parts so it takes care of
that.

The problem is that:
1. It's not an attachment. That is:
Content-Disposition: inline; filename="0001-doc_wip-diff.txt"
explicitly says it's not an attachment.

So the reason it's not listed as an attachment (per the subject of the
email) is because it *isn't* an attachment.


2. We only show the first plaintext part of messages

This is the actual problem. Our archives don't know how to merge
multiple plaintext parts when they are set to be viewed inline and not
as attachments.

That said, I notice that it's shown as an attachment in gmail as well,
even though it's not an attachment. I have no idea why :) MIME parsing
in general is black magic, but in this case it seems pretty clear that
showing it as an attachment is plain wrong. Not showing it at all,
however, is also wrong, but I'm not sure how much complexity it's
worth to handle this case, given it's the first time I've heard of the
problem.


//Magnus

Hi,

On 2022-03-26 18:41:39 +0100, Magnus Hagander wrote:
> That said, it's not as simple as a upper/lowercase. get_content_type()
> returns 'text/plain' in lowercase for both parts so it takes care of
> that.
> 
> The problem is that:
> 1. It's not an attachment. That is:
> Content-Disposition: inline; filename="0001-doc_wip-diff.txt"
> explicitly says it's not an attachment.

It's not actually that unreasonable for a doc patch ;)


> 2. We only show the first plaintext part of messages
> 
> This is the actual problem. Our archives don't know how to merge
> multiple plaintext parts when they are set to be viewed inline and not
> as attachments.
> 
> That said, I notice that it's shown as an attachment in gmail as well,
> even though it's not an attachment. I have no idea why :)

The archive code appears to actually try to handle the case of multiple inline
plain text attachements, from what I can see:
https://git.postgresql.org/gitweb/?p=pgarchives.git;a=blob;f=loader/lib/parser.py;hb=HEAD#l381
 381             # If we have already found one text/plain part, make all
 382             # further text/plain parts attachments
 383             if self.attachments_found_first_plaintext:


So there's something more than just the content-disposition: inline; going on
I think?


> MIME parsing in general is black magic, but in this case it seems pretty
> clear that showing it as an attachment is plain wrong. Not showing it at
> all, however, is also wrong, but I'm not sure how much complexity it's worth
> to handle this case, given it's the first time I've heard of the problem.

I'm not sure we'd really notice. I dimly recall encountering something like
this before and somebody else complaining about it.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2022-03-26 18:41:39 +0100, Magnus Hagander wrote:
>> MIME parsing in general is black magic, but in this case it seems pretty
>> clear that showing it as an attachment is plain wrong. Not showing it at
>> all, however, is also wrong, but I'm not sure how much complexity it's worth
>> to handle this case, given it's the first time I've heard of the problem.

> I'm not sure we'd really notice. I dimly recall encountering something like
> this before and somebody else complaining about it.

I've definitely seen other cases where the archives fail to show a portion
of a message that I can see by other means.  Whether this particular
mechanism was the cause, I can't say of course.

I recall an example recently [ digs... ] ah, here:

https://www.postgresql.org/message-id/7f6fabaa-3f8f-49ab-89ca-59fbfe633105@me.com

There is a patch in there, but it looks like the boundary line
for the preceding section might be messed up?

            regards, tom lane

At Sat, 26 Mar 2022 18:41:39 +0100, Magnus Hagander <magnus@hagander.net> wrote in 
> The problem is that:
> 1. It's not an attachment. That is:
> Content-Disposition: inline; filename="0001-doc_wip-diff.txt"
> explicitly says it's not an attachment.

RFC2183 says that the header suggests how the multipart body is
displayed, not what it is.

https://www.ietf.org/rfc/rfc2183.txt

> 4.  Summary
> 
>    Content-Disposition takes one of two values, `inline' and
>    `attachment'.  `Inline' indicates that the entity should be
>    immediately displayed to the user, whereas `attachment' means that
>    the user should take additional action to view the entity.
> 
>    The `filename' parameter can be used to suggest a filename for
>    storing the bodypart, if the user wishes to store it in an external
>    file.

Doesn't the fact that the Contnet-Disposition has the "filename"
attribute suggests the part is stored as an external content apart
from the difference between inline and attachement?

-- 
Kyotaro Horiguchi
NTT Open Source Software Center