Re: [9front] [Patch] Mail fails to send attachments

Re: [9front] [Patch] Mail fails to send attachments

[sirjofri]

Somehow this mail wasn't sent. Here it is:

20.02.2021 14:29:59 joel@sirjofri.de:

Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:

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

This patch includes that check.

I adjusted the tee function so that the second destination is ignored
if it's invalid.

sirjofri

.
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:27:35 2021 +0100
@@ -985,10 +985,12 @@
  int n;
  char buf[8*1024];

- while ((n = read(in, buf, sizeof buf)) > 0)
-   if (write(out1, buf, n) != n ||
-       write(out2, buf, n) != n)
+ while ((n = read(in, buf, sizeof buf)) > 0){
+   if (write(out1, buf, n) != n)
      break;
+   if (out2 >= 0 && write(out2, buf, n) != n)
+     break;
+ }
}

/* print the unix from line */
@@ -1086,11 +1088,14 @@
      case 0:
        close(pfd[0]);
        /* BOTCH; "From " time gets changed */
-       b = openfolder(foldername(nil, user, rcvr), time(0));
-       fd = b? Bfildes(b): -1;
-       printunixfrom(fd);
-       tee(0, pfd[1], fd);
-       write(fd, "\n", 1);
+       b = openfolder(foldername(nil, login, rcvr), time(0));
+       if (b){
+         fd = Bfildes(b);
+         printunixfrom(fd);
+         tee(0, pfd[1], fd);
+         write(fd, "\n", 1);
+       } else
+         tee(0, pfd[1], -1);
        closefolder(b);
        exits(0);
      default:

Re: [9front] [Patch] Mail fails to send attachments

[ori]

Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:
> Somehow this mail wasn't sent. Here it is:
> 
> 20.02.2021 14:29:59 joel@sirjofri.de:
> 
> Quoth sirjofri <sirjofri+ml-9front@sirjofri.de>:
> 
> > We should still check the returned file descriptor, just in case the
> > user has no mailbox directory.
> 
> This patch includes that check.
> 
> I adjusted the tee function so that the second destination is ignored
> if it's invalid.
>

Committed with some nitpicky style tweaks. Thanks. 

Re: [9front] [Bug] Mail fails to send attachments

[ori]

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.

Re: [9front] [Patch] Mail fails to send attachments

[sirjofri]

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--

Re: [9front] [Patch] Mail fails to send attachments

[hiro]

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--
>