* [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 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] [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] [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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).