9front - general discussion about 9front
 help / color / mirror / Atom feed
* [9front] [Bug] Mail fails to send attachments
@ 2021-02-19 19:56 theinicke
  2021-02-19 20:13 ` sirjofri
  0 siblings, 1 reply; 13+ messages in thread
From: theinicke @ 2021-02-19 19:56 UTC (permalink / raw)
  To: 9front

Today I have noticed that since the rewrite Mail fails to send attachments.
That is if one includes an "Attach: /some/file" line and posts it,
(if file exists) marshal suicides like so: 66286: marshal 66286: sys: write on closed pipe pc=0x25e04
leaving us with a sent mail without the attachments.

I have not been able to track down the cause yet.
Have reproduced it on a clean install updated to latest HEAD
to make sure it is not caused by any of my modifications.
Here are the symptoms:

If in /sys/src/cmd/upas/marshal/marshal.c:421 attachment function is called, then
in /sys/src/cmd/upas/marshal/marshal.c:430 the error occurs.

--
Tobias Heinicke


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [Bug] Mail fails to send attachments
  2021-02-19 19:56 [9front] [Bug] Mail fails to send attachments theinicke
@ 2021-02-19 20:13 ` sirjofri
  2021-02-19 20:54   ` Romano
                     ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: sirjofri @ 2021-02-19 20:13 UTC (permalink / raw)
  To: 9front


19.02.2021 20:56:00 theinicke@bss-wf.de:
> Today I have noticed that since the rewrite Mail fails to send 
> attachments.
> That is if one includes an "Attach: /some/file" line and posts it,
> (if file exists) marshal suicides like so: 66286: marshal 66286: sys: 
> write on closed pipe pc=0x25e04
> leaving us with a sent mail without the attachments.
>
> I have not been able to track down the cause yet.
> Have reproduced it on a clean install updated to latest HEAD
> to make sure it is not caused by any of my modifications.
> Here are the symptoms:
>
> If in /sys/src/cmd/upas/marshal/marshal.c:421 attachment function is 
> called, then
> in /sys/src/cmd/upas/marshal/marshal.c:430 the error occurs.

Same here. The same happens when using Include: instead of Attach. Ori is 
unable to reproduce this, afaik. Using marshal -8 directly works fine 
btw.

sirjofri

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [Bug] Mail fails to send attachments
  2021-02-19 20:13 ` sirjofri
@ 2021-02-19 20:54   ` Romano
  2021-02-19 21:13   ` ori
  2021-02-19 23:27   ` ori
  2 siblings, 0 replies; 13+ messages in thread
From: Romano @ 2021-02-19 20:54 UTC (permalink / raw)
  To: 9front, sirjofri

I don't get a suicide but noticed last night that Nail's comp.c adds content-type and content-enconding (IIRC; I'm in the desert right now). I removed those from the comp.c and added to my /mail/box/$user/headers and that resolved the issue, but there are still two content-type and content-encoding for the sent email, based on on the rawheader file. I haven't been able to test more--perhaps the marshal doc isn't correct re: adding headers from the headers file when none exist, and just always adds them.

On February 19, 2021 8:13:36 PM UTC, sirjofri <sirjofri+ml-9front@sirjofri.de> wrote:
>
>19.02.2021 20:56:00 theinicke@bss-wf.de:
>> Today I have noticed that since the rewrite Mail fails to send 
>> attachments.
>> That is if one includes an "Attach: /some/file" line and posts it,
>> (if file exists) marshal suicides like so: 66286: marshal 66286: sys:
>
>> write on closed pipe pc=0x25e04
>> leaving us with a sent mail without the attachments.
>>
>> I have not been able to track down the cause yet.
>> Have reproduced it on a clean install updated to latest HEAD
>> to make sure it is not caused by any of my modifications.
>> Here are the symptoms:
>>
>> If in /sys/src/cmd/upas/marshal/marshal.c:421 attachment function is 
>> called, then
>> in /sys/src/cmd/upas/marshal/marshal.c:430 the error occurs.
>
>Same here. The same happens when using Include: instead of Attach. Ori
>is 
>unable to reproduce this, afaik. Using marshal -8 directly works fine 
>btw.
>
>sirjofri

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [Bug] Mail fails to send attachments
  2021-02-19 20:13 ` sirjofri
  2021-02-19 20:54   ` Romano
@ 2021-02-19 21:13   ` ori
  2021-02-19 23:47     ` sirjofri
  2021-02-19 23:27   ` ori
  2 siblings, 1 reply; 13+ messages in thread
From: ori @ 2021-02-19 21:13 UTC (permalink / raw)
  To: 9front

Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:
> 
> 19.02.2021 20:56:00 theinicke@bss-wf.de:
> > Today I have noticed that since the rewrite Mail fails to send 
> > attachments.
> > That is if one includes an "Attach: /some/file" line and posts it,
> > (if file exists) marshal suicides like so: 66286: marshal 66286: sys: 
> > write on closed pipe pc=0x25e04
> > leaving us with a sent mail without the attachments.
> >
> > I have not been able to track down the cause yet.
> > Have reproduced it on a clean install updated to latest HEAD
> > to make sure it is not caused by any of my modifications.
> > Here are the symptoms:
> >
> > If in /sys/src/cmd/upas/marshal/marshal.c:421 attachment function is 
> > called, then
> > in /sys/src/cmd/upas/marshal/marshal.c:430 the error occurs.
> 
> Same here. The same happens when using Include: instead of Attach. Ori is 
> unable to reproduce this, afaik. Using marshal -8 directly works fine 
> btw.
> 
> sirjofri
> 

I don't know why marshal -8 would work, when
Mail wouldn't. We just invoke:

	upas/marshal -8 -S outgoing

The pipe being closed is talking to upas/send,
which is likely dying or exiting early. This
may help capture it:

diff -r 39b061370f36 sys/src/cmd/upas/marshal/marshal.c
--- a/sys/src/cmd/upas/marshal/marshal.c	Wed Feb 17 11:20:13 2021 +0100
+++ b/sys/src/cmd/upas/marshal/marshal.c	Fri Feb 19 13:08:55 2021 -0800
@@ -1172,6 +1172,7 @@
 
 	err = nil;
 	while((w = wait()) != nil){
+		fprint(2, "%d: %s\n", w->pid, w->msg);
 		if(w->pid == pid || w->pid == pgppid)
 			if(w->msg[0] != 0)
 				err = estrdup(w->msg);


If you have dtracy enabled, it may be possible
to snoop the exit status, which could help
get an idea of what's going on. Don't see a
great way to get the process name, so it'll
take some guesswork:

	dtracy 'sys:exits:entry if(arg0 != 0) {
		print probe, pid, (string)arg0
	}'



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [Bug] Mail fails to send attachments
  2021-02-19 20:13 ` sirjofri
  2021-02-19 20:54   ` Romano
  2021-02-19 21:13   ` ori
@ 2021-02-19 23:27   ` ori
  2021-02-20 13:16     ` [9front] [Patch] " sirjofri
  2 siblings, 1 reply; 13+ messages in thread
From: ori @ 2021-02-19 23:27 UTC (permalink / raw)
  To: 9front

Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:
> 
> 19.02.2021 20:56:00 theinicke@bss-wf.de:
> > Today I have noticed that since the rewrite Mail fails to send 
> > attachments.
> > That is if one includes an "Attach: /some/file" line and posts it,
> > (if file exists) marshal suicides like so: 66286: marshal 66286: sys: 
> > write on closed pipe pc=0x25e04
> > leaving us with a sent mail without the attachments.
> >
> > I have not been able to track down the cause yet.
> > Have reproduced it on a clean install updated to latest HEAD
> > to make sure it is not caused by any of my modifications.
> > Here are the symptoms:
> >
> > If in /sys/src/cmd/upas/marshal/marshal.c:421 attachment function is 
> > called, then
> > in /sys/src/cmd/upas/marshal/marshal.c:430 the error occurs.
> 
> Same here. The same happens when using Include: instead of Attach. Ori is 
> unable to reproduce this, afaik. Using marshal -8 directly works fine 
> btw.
> 
> sirjofri
> 

So, after some debugging with sirjofri, it looks
like this may have something to do with the '-S'
flag that we're passing to upas/marshal.

If you can confirm by running with 'Mail -O' to
disable saving outgoing messages, that would be
appreciated. It also gives a hint for where to
debug.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [Bug] Mail fails to send attachments
  2021-02-19 21:13   ` ori
@ 2021-02-19 23:47     ` sirjofri
  2021-02-20  0:28       ` sirjofri
  0 siblings, 1 reply; 13+ messages in thread
From: sirjofri @ 2021-02-19 23:47 UTC (permalink / raw)
  To: 9front

Hello,

I did some testing and this is what I found out:

upas/send does not fail. This is at least from what I found out using 
dtracy (no status error) and other tests. It seems only marshal fails.

Marshal doesn't always call send. If there's a pipefrom, it executes that 
instead. I tried using a minimal pipefrom (saving args and all stdin in 
file, to verify) and marshal still fails (I verified that send isn't 
called at all).

This could mean that the pipe is closed by marshal itself (by one of its 
child processes).

Marshal does fork pretty often. It even forks inside forks. I'll try to 
find out more about that tomorrow. I can imagine there's one child that 
basically closes the pipe when another child still wants to write.

I currently assume this happens somewhere in marshal.c:/^sendmail, but 
today I'm way to tired to find it.

I'll be available again tomorrow and hopefully we can find it.

sirjofri

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [Bug] Mail fails to send attachments
  2021-02-19 23:47     ` sirjofri
@ 2021-02-20  0:28       ` sirjofri
  2021-02-20  0:49         ` ori
  0 siblings, 1 reply; 13+ messages in thread
From: sirjofri @ 2021-02-20  0:28 UTC (permalink / raw)
  To: 9front

I noticed that marshal also handles the whole boundary stuff and 
attachments. The output file I get in pipefrom does not contain the data 
from the attachment.

To verify I made this pipefrom:

#!/bin/rc
echo $* > /tmp/msgargs
cat > /tmp/msgcontent

Then look at these files.

The whole thing is formatted as a multipart message, but it only contains 
the message text, no attachments.

I also looked at the history of the marshal code, I don't think the 
sendmail function is bad here.

sirjofri

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [Bug] Mail fails to send attachments
  2021-02-20  0:28       ` sirjofri
@ 2021-02-20  0:49         ` ori
  2021-02-20 13:02           ` sirjofri
  0 siblings, 1 reply; 13+ messages in thread
From: ori @ 2021-02-20  0:49 UTC (permalink / raw)
  To: 9front

Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:
> I noticed that marshal also handles the whole boundary stuff and 
> attachments. The output file I get in pipefrom does not contain the data 
> from the attachment.
> 
> To verify I made this pipefrom:
> 
> #!/bin/rc
> echo $* > /tmp/msgargs
> cat > /tmp/msgcontent
> 
> Then look at these files.
> 
> The whole thing is formatted as a multipart message, but it only contains 
> the message text, no attachments.
> 
> I also looked at the history of the marshal code, I don't think the 
> sendmail function is bad here.
> 
> sirjofri
> 

I'd look at whether we're closing the wrong fd
if we're passing '-S'.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [Bug] Mail fails to send attachments
  2021-02-20  0:49         ` ori
@ 2021-02-20 13:02           ` sirjofri
  0 siblings, 0 replies; 13+ messages in thread
From: sirjofri @ 2021-02-20 13:02 UTC (permalink / raw)
  To: 9front

Hello,

20.02.2021 01:49:09 ori@eigenstate.org:
> Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:
>> I noticed that marshal also handles the whole boundary stuff and
>> attachments. The output file I get in pipefrom does not contain the 
>> data
>> from the attachment.
>>
>> To verify I made this pipefrom:
>>
>> #!/bin/rc
>> echo $* > /tmp/msgargs
>> cat > /tmp/msgcontent
>>
>> Then look at these files.
>>
>> The whole thing is formatted as a multipart message, but it only 
>> contains
>> the message text, no attachments.
>>
>> I also looked at the history of the marshal code, I don't think the
>> sendmail function is bad here.
>>
>> sirjofri
>>
>
> I'd look at whether we're closing the wrong fd
> if we're passing '-S'.

I found out, it's something completely different.

The function foldername builds the path to the outgoing directory. For 
this, it uses the upasname environment variable (or the login name if 
it's not set).

However, I had my upasname set to my sender address (joel@sirjofri.de), 
so it builds the string: /mail/box/joel@sirjofri.de/outgoing, which of 
course cannot be opened (it doesn't exist!).

Therefore, openfolder does return a wrong Biobuf with a file descriptor 
-1, our tee function fails to write there and nothing works.

Removing my upasname variable confirmed this bug.

Now I just need to find a proper solution to this. Looking at the 
returned file descriptor could be one, writing not to user but login 
directory another.

If you want to follow my description in code, marshal.c:/^sendmail, 
switch() case 0:, if (rcvr != nil), switch() case 0, the foldername 
function call.

sirjofri

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [Patch] Mail fails to send attachments
  2021-02-19 23:27   ` ori
@ 2021-02-20 13:16     ` sirjofri
  2021-02-20 13:37       ` hiro
  0 siblings, 1 reply; 13+ messages in thread
From: sirjofri @ 2021-02-20 13:16 UTC (permalink / raw)
  To: 9front

This is a multi-part message in MIME format.
--upas-kbnxydoadswlscajpsgakobprj
Content-Disposition: inline
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit

Save outgoing files in user mailbox, fixes closed pipe bug

I noticed it is no closed pipe, but the constructed foldername for the
save location was built using the upasname variable, which can be set
to an alias.

This patch fixes that and uses the logged in username instead.
Therefore it opens the file correctly and the write works.

For example, before:

	upasname: joel@sirjofri.de
	foldername: /mail/box/joel@sirjofri.de/outgoing

after:

	upasname: joel@sirjofri.de
	foldername: /mail/box/sirjofri/outgoing

If an attachment is here, then the patch works (on my machine, at
least).

We should still check the returned file descriptor, just in case the
user has no mailbox directory.

sirjofri

.
--upas-kbnxydoadswlscajpsgakobprj
Content-Disposition: inline
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit

diff -r 297026d9dc30 sys/src/cmd/upas/marshal/marshal.c
--- a/sys/src/cmd/upas/marshal/marshal.c	Thu Feb 18 21:40:30 2021 +0100
+++ b/sys/src/cmd/upas/marshal/marshal.c	Sat Feb 20 14:09:26 2021 +0100
@@ -1086,7 +1086,7 @@
 			case 0:
 				close(pfd[0]);
 				/* BOTCH; "From " time gets changed */
-				b = openfolder(foldername(nil, user, rcvr), time(0));
+				b = openfolder(foldername(nil, login, rcvr), time(0));
 				fd = b? Bfildes(b): -1;
 				printunixfrom(fd);
 				tee(0, pfd[1], fd);
--upas-kbnxydoadswlscajpsgakobprj--

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [Patch] Mail fails to send attachments
  2021-02-20 13:16     ` [9front] [Patch] " sirjofri
@ 2021-02-20 13:37       ` hiro
  2021-02-20 14:03         ` [9front] bad headers in mail (was: Mail attachment bug) sirjofri
  0 siblings, 1 reply; 13+ messages in thread
From: hiro @ 2021-02-20 13:37 UTC (permalink / raw)
  To: 9front

gmail still displays your mails weirdly, might be unrelated but not sure:

In-Reply-To: <E07C376E4275255C9E2473CC13E5C94B@eigenstate.org>
MIME-Version: 1.0
Content-Type: multipart/mixed;
boundary="upas-kbnxydoadswlscajpsgakobprj" <<< this seems to be
ignored by my gmail that displays the message. is it too early?
shouldn't it be the last line in the headers?
List-ID: <9front.9front.org> <<< obviously this got added by 9front
mailing list... might be a harmful interaction with your header... ?
List-Help: <http://lists.9front.org>
X-Glyph: ➈
X-Bullshit: encrypted interface-aware base high-performance frontend
Subject: Re: [9front] [Patch] Mail fails to send attachments
Reply-To: 9front@9front.org
Precedence: bulk
<< this empty line marks the beginning of the message...
This is a multi-part message in MIME format. << ...which is why gmail
displays this
--upas-kbnxydoadswlscajpsgakobprj <<<and ignores this, displaying this
and the rest of the body as plain text including those headers.
Content-Disposition: inline
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit


On 2/20/21, sirjofri <sirjofri+ml-9front@sirjofri.de> wrote:
> This is a multi-part message in MIME format.
> --upas-kbnxydoadswlscajpsgakobprj
> Content-Disposition: inline
> Content-Type: text/plain; charset="US-ASCII"
> Content-Transfer-Encoding: 7bit
>
> Save outgoing files in user mailbox, fixes closed pipe bug
>
> I noticed it is no closed pipe, but the constructed foldername for the
> save location was built using the upasname variable, which can be set
> to an alias.
>
> This patch fixes that and uses the logged in username instead.
> Therefore it opens the file correctly and the write works.
>
> For example, before:
>
> 	upasname: joel@sirjofri.de
> 	foldername: /mail/box/joel@sirjofri.de/outgoing
>
> after:
>
> 	upasname: joel@sirjofri.de
> 	foldername: /mail/box/sirjofri/outgoing
>
> If an attachment is here, then the patch works (on my machine, at
> least).
>
> We should still check the returned file descriptor, just in case the
> user has no mailbox directory.
>
> sirjofri
>
> .
> --upas-kbnxydoadswlscajpsgakobprj
> Content-Disposition: inline
> Content-Type: text/plain; charset="US-ASCII"
> Content-Transfer-Encoding: 7bit
>
> diff -r 297026d9dc30 sys/src/cmd/upas/marshal/marshal.c
> --- a/sys/src/cmd/upas/marshal/marshal.c	Thu Feb 18 21:40:30 2021 +0100
> +++ b/sys/src/cmd/upas/marshal/marshal.c	Sat Feb 20 14:09:26 2021 +0100
> @@ -1086,7 +1086,7 @@
>  			case 0:
>  				close(pfd[0]);
>  				/* BOTCH; "From " time gets changed */
> -				b = openfolder(foldername(nil, user, rcvr), time(0));
> +				b = openfolder(foldername(nil, login, rcvr), time(0));
>  				fd = b? Bfildes(b): -1;
>  				printunixfrom(fd);
>  				tee(0, pfd[1], fd);
> --upas-kbnxydoadswlscajpsgakobprj--
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] bad headers in mail (was: Mail attachment bug)
  2021-02-20 13:37       ` hiro
@ 2021-02-20 14:03         ` sirjofri
  2021-02-20 14:10           ` hiro
  0 siblings, 1 reply; 13+ messages in thread
From: sirjofri @ 2021-02-20 14:03 UTC (permalink / raw)
  To: hiro


20.02.2021 14:37:42 hiro <23hiro@gmail.com>:
> gmail still displays your mails weirdly, might be unrelated but not 
> sure:

Hey hiro,

I sent mails to myself and looked at the headers, also at what you sent.

If the multipart header must be the last then my sent mail is correct (at 
least from what I can find out/what upas/send receives from marshal). I 
can imagine it's the mailing list adding headers in weird places?

It also might be different using Include: and Attach:. Can I send you two 
mails, one with Include:d attachment, the other Attach:ed?

sirjofri

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] bad headers in mail (was: Mail attachment bug)
  2021-02-20 14:03         ` [9front] bad headers in mail (was: Mail attachment bug) sirjofri
@ 2021-02-20 14:10           ` hiro
  0 siblings, 0 replies; 13+ messages in thread
From: hiro @ 2021-02-20 14:10 UTC (permalink / raw)
  To: 9front

tbh i only skimmed the RFC, and the wrong one at that.
i have seen another one now that does the same thing where the
multipart header is also not the last line in the headers, and it gets
displayed properly.

you can send me those mails, yeah.

it might be that your mail is actually compliant, but in a rare form
that gmail ignores.
i can not make sense of it tbh.

maybe the in-line stuff forces gmail to show them as is and with the headers?

On 2/20/21, sirjofri <sirjofri+ml-9front@sirjofri.de> wrote:
>
> 20.02.2021 14:37:42 hiro <23hiro@gmail.com>:
>> gmail still displays your mails weirdly, might be unrelated but not
>> sure:
>
> Hey hiro,
>
> I sent mails to myself and looked at the headers, also at what you sent.
>
> If the multipart header must be the last then my sent mail is correct (at
> least from what I can find out/what upas/send receives from marshal). I
> can imagine it's the mailing list adding headers in weird places?
>
> It also might be different using Include: and Attach:. Can I send you two
> mails, one with Include:d attachment, the other Attach:ed?
>
> sirjofri
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-02-20 14:18 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-19 19:56 [9front] [Bug] Mail fails to send attachments theinicke
2021-02-19 20:13 ` sirjofri
2021-02-19 20:54   ` Romano
2021-02-19 21:13   ` ori
2021-02-19 23:47     ` sirjofri
2021-02-20  0:28       ` sirjofri
2021-02-20  0:49         ` ori
2021-02-20 13:02           ` sirjofri
2021-02-19 23:27   ` ori
2021-02-20 13:16     ` [9front] [Patch] " sirjofri
2021-02-20 13:37       ` hiro
2021-02-20 14:03         ` [9front] bad headers in mail (was: Mail attachment bug) sirjofri
2021-02-20 14:10           ` hiro

9front - general discussion about 9front

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/9front

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 9front 9front/ http://inbox.vuxu.org/9front \
		9front@9front.org
	public-inbox-index 9front

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.9front


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git