sendbug(1) patch

[Dasn-6]

Hi,

I found some people suffer from the failure of sendbug(1). Because most
of them did not (or had no permission to) configure their sendmail(1)
properly, I suppose.

The following patch in order to solve this problem by adding a new
environment variable, PR_AGENT, which allows users to specify their own
MTA instead of the default sendmail(1). For example:

$ export PR_AGENT="/usr/local/bin/msmtp -a my_obsd_account -t"
$ sendbug


diff -u /usr/src/gnu/usr.bin/sendbug/sendbug.1 sendbug/sendbug.1
--- /usr/src/gnu/usr.bin/sendbug/sendbug.1 Tue Aug  1 18:10:33 2006
+++ sendbug/sendbug.1 Sun Apr 13 10:12:21 2008
@@ -123,7 +123,7 @@
 .Ox
 bugs database.
 .Sh ENVIRONMENT
-.Bl -tag -width "PR_FORM"
+.Bl -tag -width "PR_AGENT"
 .It Ev EDITOR
 Specifies the editor to invoke on the template.
 The default editor is
@@ -132,6 +132,9 @@
 Used as the file name of the template for your problem-report editing session.
 You can use this to start with a partially completed form (for example,
 a form with the identification fields already completed).
+.It Ev PR_AGENT
+The absolute path to the program which override the default Mail Transfer Agent
+to send the PR.
 .El
 .Sh HOW TO FILL OUT A PROBLEM REPORT

diff -u /usr/src/gnu/usr.bin/sendbug/sendbug.sh sendbug/sendbug.sh
--- /usr/src/gnu/usr.bin/sendbug/sendbug.sh Tue Nov 30 05:57:45 2004
+++ sendbug/sendbug.sh Sun Apr 13 09:54:21 2008
@@ -57,11 +57,17 @@
 
 # What mailer to use.  This must come after the config file, since it is
 # host-dependent.
-if [ -f /usr/sbin/sendmail ]; then
-  MAIL_AGENT="/usr/sbin/sendmail -oi -t"
+if [ X"$PR_AGENT" = X"" ]; then
+ if [ -f /usr/sbin/sendmail ]; then
+  MAIL_AGENT="/usr/sbin/sendmail -oi -t"
+ else
+  MAIL_AGENT="/usr/lib/sendmail -oi -t"
+ fi
 else
-  MAIL_AGENT="/usr/lib/sendmail -oi -t"
+ # A user defined mail transfer agent.
+ MAIL_AGENT="$PR_AGENT"
 fi
+
 MAILER=`echo $MAIL_AGENT | sed -e 's, .*,,'`
 if [ ! -f "$MAILER" ]; then
   echo "$COMMAND: Cannot find mail program \"$MAILER\"."
--
Dasn

[Ray-72]

Sendbug was rewritten some time ago, so your patch doesn't apply, sorry.

I was unaware of problems people had with sendmail, is this a  
widespread problem?

-Ray-

On Apr 12, 2008, at 10:51 PM, Dasn <dasn@...> wrote:

> Hi,
>
> I found some people suffer from the failure of sendbug(1). Because  
> most
> of them did not (or had no permission to) configure their sendmail(1)
> properly, I suppose.
>
> The following patch in order to solve this problem by adding a new
> environment variable, PR_AGENT, which allows users to specify their  
> own
> MTA instead of the default sendmail(1). For example:
>
> $ export PR_AGENT="/usr/local/bin/msmtp -a my_obsd_account -t"
> $ sendbug
>
>
> diff -u /usr/src/gnu/usr.bin/sendbug/sendbug.1 sendbug/sendbug.1
> --- /usr/src/gnu/usr.bin/sendbug/sendbug.1    Tue Aug  1 18:10:33 2006
> +++ sendbug/sendbug.1    Sun Apr 13 10:12:21 2008
> @@ -123,7 +123,7 @@
> .Ox
> bugs database.
> .Sh ENVIRONMENT
> -.Bl -tag -width "PR_FORM"
> +.Bl -tag -width "PR_AGENT"
> .It Ev EDITOR
> Specifies the editor to invoke on the template.
> The default editor is
> @@ -132,6 +132,9 @@
> Used as the file name of the template for your problem-report  
> editing session.
> You can use this to start with a partially completed form (for  
> example,
> a form with the identification fields already completed).
> +.It Ev PR_AGENT
> +The absolute path to the program which override the default Mail  
> Transfer Agent
> +to send the PR.
> .El
> .Sh HOW TO FILL OUT A PROBLEM REPORT
>
> diff -u /usr/src/gnu/usr.bin/sendbug/sendbug.sh sendbug/sendbug.sh
> --- /usr/src/gnu/usr.bin/sendbug/sendbug.sh    Tue Nov 30 05:57:45  
> 2004
> +++ sendbug/sendbug.sh    Sun Apr 13 09:54:21 2008
> @@ -57,11 +57,17 @@
> # What mailer to use.  This must come after the config file, since  
> it is
> # host-dependent.
> -if [ -f /usr/sbin/sendmail ]; then
> -  MAIL_AGENT="/usr/sbin/sendmail -oi -t"
> +if [ X"$PR_AGENT" = X"" ]; then
> +    if [ -f /usr/sbin/sendmail ]; then
> +      MAIL_AGENT="/usr/sbin/sendmail -oi -t"
> +    else
> +      MAIL_AGENT="/usr/lib/sendmail -oi -t"
> +    fi
> else
> -  MAIL_AGENT="/usr/lib/sendmail -oi -t"
> +    # A user defined mail transfer agent.
> +    MAIL_AGENT="$PR_AGENT"
> fi
> +
> MAILER=`echo $MAIL_AGENT | sed -e 's, .*,,'`
> if [ ! -f "$MAILER" ]; then
>  echo "$COMMAND: Cannot find mail program \"$MAILER\"."
> --
> Dasn

[Ian Darwin]

> You can use this to start with a partially completed form (for example,
> a form with the identification fields already completed).
> +.It Ev PR_AGENT
> +The absolute path to the program which override the default Mail
> Transfer Agent
> +to send the PR.

Given how it's used in the script, a better wording would be smth like:

A string containing the full path to an alternate Mail Transfer Agent
along with any options that this MTA requires.

[Dasn-6]

On 12/04/08 23:50 -0400, Ray wrote:
>Sendbug was rewritten some time ago, so your patch doesn't apply, sorry.
>
>I was unaware of problems people had with sendmail, is this a  
>widespread problem?
>
>-Ray-
>

I'm still in obsd 4.1, thanks for your pointer.

When I use the sendmail to deliver messages to some mail servers, I got
the message returned with errors like:

> Diagnostic-Code: SMTP; 550 The IP address x.x.x.x has been listed on a
> realtime blacklist ...

I suppose my ISP's NAT allocated a notorious address for me. So I have
to use an alternative SMTP server to send the mail.

The following patch against the new (rewritten) sendbug (v4.2), with the
PR_AGENT feature and a few minor fixes.

Anyway, another use of the 'PR_AGENT' feature is, to help test sendbug's
send-outs, like:

$ export PR_AGENT="tee mail.out"
$ ./sendbug

:)

diff -u /usr/src/usr.bin/sendbug/sendbug.1 sendbug/sendbug.1
--- /usr/src/usr.bin/sendbug/sendbug.1 Fri Jun  1 03:20:16 2007
+++ sendbug/sendbug.1 Tue Apr 15 18:47:53 2008
@@ -91,6 +91,8 @@
 Filename of PR form to use instead of using the built-in form.
 Such a PR form can be partially pre-completed to make the
 process faster.
+.It Ev PR_AGENT
+Specifies an alternate Mail Transfer Agent to send the PR
 .It Ev TMPDIR
 Specifies a directory for temporary files to be created.
 The default is
@@ -103,7 +105,8 @@
 .El
 .Sh SEE ALSO
 .Xr crash 8 ,
-.Xr dmesg 8
+.Xr dmesg 8 ,
+.Xr sendmail 8
 .Sh AUTHORS
 .Nm
 was written from scratch for the public domain by

diff -u /usr/src/usr.bin/sendbug/sendbug.c sendbug/sendbug.c
--- /usr/src/usr.bin/sendbug/sendbug.c Fri Jan  4 08:50:09 2008
+++ sendbug/sendbug.c Tue Apr 15 18:53:29 2008
@@ -303,39 +303,81 @@
 sendmail(const char *pathname)
 {
  int filedes[2];
+ pid_t pid;
 
  if (pipe(filedes) == -1) {
  warn("pipe: unsent report in %s", pathname);
  return (-1);
  }
- switch (fork()) {
+
+ pid = fork();
+
+ switch (pid) {
  case -1:
  warn("fork error: unsent report in %s",
     pathname);
  return (-1);
  case 0:
  close(filedes[1]);
- if (dup2(filedes[0], STDIN_FILENO) == -1) {
- warn("dup2 error: unsent report in %s",
-    pathname);
- return (-1);
+
+ if (filedes[0] != STDIN_FILENO) { /* defensive measure */
+ if (dup2(filedes[0], STDIN_FILENO) == -1) {
+ warn("dup2 error: unsent report in %s",
+ pathname);
+ return (-1);
+ }
+ close(filedes[0]);
  }
+
+ char *mta_cmd;
+
+ if ( (mta_cmd = getenv("PR_AGENT")) == NULL ) {
+ execl("/usr/sbin/sendmail", "sendmail",
+ "-oi", "-t", (void *)NULL);
+ } else {
+ execl(_PATH_BSHELL, "sh", "-c", mta_cmd,
+ (void *) NULL);
+ }
+
+ /* shouldn't reach here */
+ warn("exec error: unsent report in %s", pathname);
+ _exit(127);
+
+ default:
  close(filedes[0]);
- execl("/usr/sbin/sendmail", "sendmail",
-    "-oi", "-t", (void *)NULL);
- warn("sendmail error: unsent report in %s",
-    pathname);
- return (-1);
- default:
- close(filedes[0]);
  /* Pipe into sendmail. */
  if (send_file(pathname, filedes[1]) == -1) {
+ wantcleanup = 0;
  warn("send_file error: unsent report in %s",
     pathname);
  return (-1);
  }
+
  close(filedes[1]);
- wait(NULL);
+
+ /* We should check the return value of MTA to see whether the
+ * PR has been sent successfully, if not, save the user's work
+ * (unsent report).
+ */
+
+ int status, ret_child = 0; /* the return value of MTA */
+ while (waitpid(pid, &status, 0) == -1)
+ if (errno != EINTR)
+ return(-1);
+
+ ret_child = WIFEXITED(status) ? WEXITSTATUS(status) : -1;
+
+ if (ret_child != 0) {
+ wantcleanup = 0;
+                        /*
+ * warn("sendmail error: unsent report in %s",
+ *                 pathname);
+                         */
+ fprintf(stderr, "MTA error (%d returned):"
+      " unsent report in %s", ret_child,
+ pathname);
+ return(-1);
+ }
  break;
  }
  return (0);
@@ -467,7 +509,9 @@
  if (ep)
  copylen = sp - buf;
  else
- copylen = len;
+ /* The final newline has been stripped,
+ * so minus 1.  */
+ copylen = len - 1;
  if (atomicio(vwrite, dst, buf, copylen) != copylen ||
     atomicio(vwrite, dst, "\n", 1) != 1)
  goto end;

--
Dasn

[Ray Lai]

On Tue, Apr 15, 2008 at 7:43 AM, Dasn <dasn@...> wrote:

> On 12/04/08 23:50 -0400, Ray wrote:
>
> > Sendbug was rewritten some time ago, so your patch doesn't apply, sorry.
> >
> > I was unaware of problems people had with sendmail, is this a  widespread
> problem?
> >
> > -Ray-
> >
> >
>
>  I'm still in obsd 4.1, thanks for your pointer.
>
>  When I use the sendmail to deliver messages to some mail servers, I got
>  the message returned with errors like:
>
>
> > Diagnostic-Code: SMTP; 550 The IP address x.x.x.x has been listed on a
> > realtime blacklist ...
> >
>
>  I suppose my ISP's NAT allocated a notorious address for me. So I have
>  to use an alternative SMTP server to send the mail.
>
>  The following patch against the new (rewritten) sendbug (v4.2), with the
>  PR_AGENT feature and a few minor fixes.
>  Anyway, another use of the 'PR_AGENT' feature is, to help test sendbug's
>  send-outs, like:
>
>  $ export PR_AGENT="tee mail.out"
>  $ ./sendbug

This is unnecessary, it already saves a copy of the report when you
abort (see sendbug(1)):

     Once the PR is filled out, the user exits the editor and is presented
     with a choice: a)bort, e)dit, or s)end.  If abort is selected, the report
     is not deleted and the pathname of the report is printed.  If edit is se-
     lected, the user is given a chance to re-edit the report.

-Ray-

[Dasn-6]

On 15/04/08 10:56 -0400, Ray wrote:
>
>This is unnecessary, it already saves a copy of the report when you
>abort (see sendbug(1)):
>
>     Once the PR is filled out, the user exits the editor and is presented
>     with a choice: a)bort, e)dit, or s)end.  If abort is selected, the report
>     is not deleted and the pathname of the report is printed.  If edit is se-
>     lected, the user is given a chance to re-edit the report.
>

That's different, the temp file is not exactly the same as what is going
to be sent to the pipe. What if there's a bug in the 'send_file'
function?

--
Dasn

[Ray Lai]

You are requesting a change in sendbug because your IP address is
blacklisted.  I think the proper solution to this problem does not
involve sendbug at all.

>  That's different, the temp file is not exactly the same as what is going
>  to be sent to the pipe.

sendbug sends a copy of the final report to the local user.

> What if there's a bug in the 'send_file' function?

1) Are there any?
2) Fix them.
3) We can't keep adding code (and bugs) for disaster scenarios.
4) There are debuggers for this purpose.
5) Your local user already gets a copy, whether it is by aborting or
by sending the report.

-Ray-

[Dasn-6]

On 16/04/08 00:07 -0400, Ray wrote:
>You are requesting a change in sendbug because your IP address is
>blacklisted.  I think the proper solution to this problem does not
>involve sendbug at all.

This is how I currently use the sendbug:
1) sendbug
2) choose a)bort
3) vi /tmp/p.XXXXXX
4) remove '>SENDBUG' and other comments by myself
5) send it via msmtp

>
>>  That's different,  the temp file is not exactly the same as what is
>>  going to be sent to the pipe.
>
>sendbug sends a copy of the final report to the local user.

Not the same thing, see below.

>> What if there's a bug in the 'send_file' function?
>
>1) Are there any?
>2) Fix them.

Please check the patch below.

>3) We can't keep adding code (and bugs) for disaster scenarios.
>4) There are debuggers for this purpose.
>5) Your local user already gets a copy, whether it is by aborting or
>by sending the report.

Now I can see that...we are totally misunderstanding each other. It's my
fault not to point out the problem clearly.

In the 'send_file' function, I found it sends each line + '\0' + '\n' to
the pipe, (I think it's a bug), and the sendmail can happily accept it,
while other MTAs will report an error. So your idea about Cc: to a local
user will not reveal this problem, because the outputs to the pipe by
'sendbug' will be processed (masked) by sendmail. So the copy of local
user is not the same what 'sendbug' writes to the pipe.

Please note that I am not meaning adding this feature is in order to
only test the function, it's just an involuntary-side of this feature.

Originally, IMHO, sendbug will be better if only produce its raw PR mail
file for users, and let the users decide how to send it (use sendmail or
others), that is why I suggested the PR_AGENT feature.


--- /usr/src/usr.bin/sendbug/sendbug.c Fri Jan  4 08:50:09 2008
+++ sendbug.c Wed Apr 16 19:32:49 2008
@@ -315,10 +315,12 @@
  return (-1);
  case 0:
  close(filedes[1]);
- if (dup2(filedes[0], STDIN_FILENO) == -1) {
- warn("dup2 error: unsent report in %s",
-    pathname);
- return (-1);
+ if (filedes[0] != STDIN_FILENO) { /* defensive measure */
+ if (dup2(filedes[0], STDIN_FILENO) == -1) {
+ warn("dup2 error: unsent report in %s",
+ pathname);
+ return (-1);
+ }
  }
  close(filedes[0]);
  execl("/usr/sbin/sendmail", "sendmail",
@@ -330,6 +332,8 @@
  close(filedes[0]);
  /* Pipe into sendmail. */
  if (send_file(pathname, filedes[1]) == -1) {
+ wantcleanup = 0; /* without this,
+    temp won't be saved */
  warn("send_file error: unsent report in %s",
     pathname);
  return (-1);
@@ -467,7 +471,9 @@
  if (ep)
  copylen = sp - buf;
  else
- copylen = len;
+ /* The final '\n' has been stripped, so minus 1,
+ * otherwise the '\0' will be sent to pipe */
+ copylen = len - 1;
  if (atomicio(vwrite, dst, buf, copylen) != copylen ||
     atomicio(vwrite, dst, "\n", 1) != 1)
  goto end;

--
Dasn

[Ray Lai]

On Wed, Apr 16, 2008 at 9:50 AM, Dasn <dasn@...> wrote:
>  Now I can see that...we are totally misunderstanding each other. It's my
>  fault not to point out the problem clearly.
>  In the 'send_file' function, I found it sends each line + '\0' + '\n' to
>  the pipe, (I think it's a bug), and the sendmail can happily accept it,
>  while other MTAs will report an error. So your idea about Cc: to a local
>  user will not reveal this problem, because the outputs to the pipe by
>  'sendbug' will be processed (masked) by sendmail. So the copy of local
>  user is not the same what 'sendbug' writes to the pipe.

Thanks for reporting the bug, a fix for this and another bug was committed.

>  Please note that I am not meaning adding this feature is in order to
>  only test the function, it's just an involuntary-side of this feature.
>  Originally, IMHO, sendbug will be better if only produce its raw PR mail
>  file for users, and let the users decide how to send it (use sendmail or
>  others), that is why I suggested the PR_AGENT feature.

I still don't think this should be added, sorry.

-Ray-

[Stuart Henderson]

On 2008/04/16 21:51, Dasn wrote:
> Originally, IMHO, sendbug will be better if only produce its raw PR mail
> file for users, and let the users decide how to send it (use sendmail or
> others), that is why I suggested the PR_AGENT feature.

We already have a mechanism for this; mailer.conf(5)

[Dasn-6]

On 19/04/08 10:43 +0100, Stuart Henderson wrote:
>On 2008/04/16 21:51, Dasn wrote:
>> Originally, IMHO, sendbug will be better if only produce its raw PR mail
>> file for users, and let the users decide how to send it (use sendmail or
>> others), that is why I suggested the PR_AGENT feature.
>
>We already have a mechanism for this; mailer.conf(5)
>
Thanks for the info, but
1) A normal user do not have permissions to change it.
2) I think mailwrapper requires a fully functional MTA. That is
something like msmtp, which is just an SMTP client, might screw up the
whole system if it was invoked by the /usr/sbin/sendmail.

--
Dasn