milter macro names

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

milter macro names

Matthias Schneider
Hi,

I just tried to upgrade our postfix instances from 2.11 to 3.1. This
broke our milter that is expecting macro with name "i" but we got "{i}".
Could we make this configurable?

postfix 2.11:

.b...b.....rDRi.3vN6Xk4v7ZzDwHS.{rcpt_addr}.matthias.schneider@

postfix 3.1:
6ZH.6ZH....kDR{i}.3vN6VP5LlPz24hjY.{rcpt_addr}.matthias.schneider@


Thanks
Matthias Schneider

Reply | Threaded
Open this post in threaded view
|

Re: milter macro names (potential patch)

Viktor Dukhovni
On Tue, Feb 14, 2017 at 05:54:07PM +0100, Matthias Schneider wrote:

> I just tried to upgrade our postfix instances from 2.11 to 3.1. This broke
> our milter that is expecting macro with name "i" but we got "{i}".
> Could we make this configurable?

It may be simplest to revert to the original (braceless) form of
single-letter macros, I think that's more typically expected.

From the Sendmail book:

    Macros may have single-character names or multicharacter names.
    Multicharacter names must always be enclosed in curly braces.
    Single-character names may be enclosed in curly braces if you
    desire. Prior to V8.7 you could use single characters only
    without curly braces.

    [ This does suggest that your milter application ought to be
      able to deal with either form, but theory and practice do
      at times differ. ]

Does the patch below resolve your issue?

diff --git a/src/milter/milter.c b/src/milter/milter.c
index 64836d4..bf2760a 100644
--- a/src/milter/milter.c
+++ b/src/milter/milter.c
@@ -333,16 +333,17 @@ static ARGV *milter_macro_lookup(MILTERS *milters, const char *macro_names)
     VSTRING *canon_buf = vstring_alloc(20);
     const char *value;
     const char *name;
+    const char *cname;
 
     while ((name = mystrtok(&cp, CHARS_COMMA_SP)) != 0) {
  if (msg_verbose)
     msg_info("%s: \"%s\"", myname, name);
  if (*name != '{') /* } */
-    name = STR(vstring_sprintf(canon_buf, "{%s}", name));
- if ((value = milters->mac_lookup(name, milters->mac_context)) != 0) {
+    cname = STR(vstring_sprintf(canon_buf, "{%s}", name));
+ if ((value = milters->mac_lookup(cname, milters->mac_context)) != 0) {
     if (msg_verbose)
  msg_info("%s: result \"%s\"", myname, value);
-    argv_add(argv, name, value, (char *) 0);
+    argv_add(argv, name[1] == '\0' ? name : cname, value, (char *) 0);
  } else if (milters->macro_defaults != 0
      && (value = htable_find(milters->macro_defaults, name)) != 0) {
     if (msg_verbose)

--
        Viktor.
Reply | Threaded
Open this post in threaded view
|

Re: milter macro names (potential patch)

Matthias Schneider
Hi Viktor,

i applyed the patch and after connecting to port 25 i'll get:

postfix/master[15476]: warning: process /usr/lib/postfix/sbin/smtpd pid 15593 killed by signal 11
postfix/master[15476]: warning: /usr/lib/postfix/sbin/smtpd: bad command startup -- throttling

my code:

 /* milter_macro_lookup - look up macros */

  static ARGV *milter_macro_lookup(MILTERS *milters, const char *macro_names)
  {
      const char *myname = "milter_macro_lookup";
      char   *saved_names = mystrdup(macro_names);
      char   *cp = saved_names;
      ARGV   *argv = argv_alloc(10);
      VSTRING *canon_buf = vstring_alloc(20);
      const char *value;
      const char *name;
      const char *cname;

      while ((name = mystrtok(&cp, CHARS_COMMA_SP)) != 0) {
          if (msg_verbose)
              msg_info("%s: \"%s\"", myname, name);
          if (*name != '{')                       /* } */
              cname = STR(vstring_sprintf(canon_buf, "%s", name));
          if ((value = milters->mac_lookup(cname, milters->mac_context)) != 0) {
              if (msg_verbose)
                  msg_info("%s: result \"%s\"", myname, value);
              argv_add(argv, name[1] == '\0' ? name : cname, value, (char *) 0);
          } else if (milters->macro_defaults != 0
               && (value = htable_find(milters->macro_defaults, name)) != 0) {
              if (msg_verbose)
                  msg_info("%s: using default \"%s\"", myname, value);
              argv_add(argv, name, value, (char *) 0);
          }
      }
      myfree(saved_names);
      vstring_free(canon_buf);
      return (argv);
  }


Thank you,
Matthias Schneider

----- Urspr√ľngliche Mail -----
Von: "Viktor Dukhovni" <[hidden email]>
An: [hidden email]
Gesendet: Dienstag, 14. Februar 2017 20:43:42
Betreff: Re: milter macro names (potential patch)

On Tue, Feb 14, 2017 at 05:54:07PM +0100, Matthias Schneider wrote:

> I just tried to upgrade our postfix instances from 2.11 to 3.1. This broke
> our milter that is expecting macro with name "i" but we got "{i}".
> Could we make this configurable?

It may be simplest to revert to the original (braceless) form of
single-letter macros, I think that's more typically expected.

From the Sendmail book:

    Macros may have single-character names or multicharacter names.
    Multicharacter names must always be enclosed in curly braces.
    Single-character names may be enclosed in curly braces if you
    desire. Prior to V8.7 you could use single characters only
    without curly braces.

    [ This does suggest that your milter application ought to be
      able to deal with either form, but theory and practice do
      at times differ. ]

Does the patch below resolve your issue?

diff --git a/src/milter/milter.c b/src/milter/milter.c
index 64836d4..bf2760a 100644
--- a/src/milter/milter.c
+++ b/src/milter/milter.c
@@ -333,16 +333,17 @@ static ARGV *milter_macro_lookup(MILTERS *milters, const char *macro_names)
     VSTRING *canon_buf = vstring_alloc(20);
     const char *value;
     const char *name;
+    const char *cname;
 
     while ((name = mystrtok(&cp, CHARS_COMMA_SP)) != 0) {
  if (msg_verbose)
     msg_info("%s: \"%s\"", myname, name);
  if (*name != '{') /* } */
-    name = STR(vstring_sprintf(canon_buf, "{%s}", name));
- if ((value = milters->mac_lookup(name, milters->mac_context)) != 0) {
+    cname = STR(vstring_sprintf(canon_buf, "{%s}", name));
+ if ((value = milters->mac_lookup(cname, milters->mac_context)) != 0) {
     if (msg_verbose)
  msg_info("%s: result \"%s\"", myname, value);
-    argv_add(argv, name, value, (char *) 0);
+    argv_add(argv, name[1] == '\0' ? name : cname, value, (char *) 0);
  } else if (milters->macro_defaults != 0
      && (value = htable_find(milters->macro_defaults, name)) != 0) {
     if (msg_verbose)

--
        Viktor.
Reply | Threaded
Open this post in threaded view
|

Re: milter macro names (potential patch)

Viktor Dukhovni
On Tue, Feb 14, 2017 at 09:43:25PM +0100, Matthias Schneider wrote:
> Hi Viktor,
>
> i applyed the patch and after connecting to port 25 i'll get:
>

Yes, sorry, the original patch is buggy, it fails to initialize
"cname" for already canonical (enclosed in {}) multi-char names.
Try this one instead:

diff --git a/src/milter/milter.c b/src/milter/milter.c
index 64836d4..571db7a 100644
--- a/src/milter/milter.c
+++ b/src/milter/milter.c
@@ -333,16 +333,19 @@ static ARGV *milter_macro_lookup(MILTERS *milters, const char *macro_names)
     VSTRING *canon_buf = vstring_alloc(20);
     const char *value;
     const char *name;
+    const char *cname;
 
     while ((name = mystrtok(&cp, CHARS_COMMA_SP)) != 0) {
  if (msg_verbose)
     msg_info("%s: \"%s\"", myname, name);
  if (*name != '{') /* } */
-    name = STR(vstring_sprintf(canon_buf, "{%s}", name));
- if ((value = milters->mac_lookup(name, milters->mac_context)) != 0) {
+    cname = STR(vstring_sprintf(canon_buf, "{%s}", name));
+ else
+    cname = name;
+ if ((value = milters->mac_lookup(cname, milters->mac_context)) != 0) {
     if (msg_verbose)
  msg_info("%s: result \"%s\"", myname, value);
-    argv_add(argv, name, value, (char *) 0);
+    argv_add(argv, name[1] == '\0' ? name : cname, value, (char *) 0);
  } else if (milters->macro_defaults != 0
      && (value = htable_find(milters->macro_defaults, name)) != 0) {
     if (msg_verbose)

--
        Viktor.
Reply | Threaded
Open this post in threaded view
|

Re: milter macro names (potential patch)

Matthias Schneider
Hi Viktor,

this patch works like a charm!
Any chance to get this back into next stable release?

Best regards
Matthias Schneider


----- Urspr√ľngliche Mail -----
Von: "Viktor Dukhovni" <[hidden email]>
An: [hidden email]
Gesendet: Dienstag, 14. Februar 2017 21:54:44
Betreff: Re: milter macro names (potential patch)

On Tue, Feb 14, 2017 at 09:43:25PM +0100, Matthias Schneider wrote:
> Hi Viktor,
>
> i applyed the patch and after connecting to port 25 i'll get:
>

Yes, sorry, the original patch is buggy, it fails to initialize
"cname" for already canonical (enclosed in {}) multi-char names.
Try this one instead:

diff --git a/src/milter/milter.c b/src/milter/milter.c
index 64836d4..571db7a 100644
--- a/src/milter/milter.c
+++ b/src/milter/milter.c
@@ -333,16 +333,19 @@ static ARGV *milter_macro_lookup(MILTERS *milters, const char *macro_names)
     VSTRING *canon_buf = vstring_alloc(20);
     const char *value;
     const char *name;
+    const char *cname;
 
     while ((name = mystrtok(&cp, CHARS_COMMA_SP)) != 0) {
  if (msg_verbose)
     msg_info("%s: \"%s\"", myname, name);
  if (*name != '{') /* } */
-    name = STR(vstring_sprintf(canon_buf, "{%s}", name));
- if ((value = milters->mac_lookup(name, milters->mac_context)) != 0) {
+    cname = STR(vstring_sprintf(canon_buf, "{%s}", name));
+ else
+    cname = name;
+ if ((value = milters->mac_lookup(cname, milters->mac_context)) != 0) {
     if (msg_verbose)
  msg_info("%s: result \"%s\"", myname, value);
-    argv_add(argv, name, value, (char *) 0);
+    argv_add(argv, name[1] == '\0' ? name : cname, value, (char *) 0);
  } else if (milters->macro_defaults != 0
      && (value = htable_find(milters->macro_defaults, name)) != 0) {
     if (msg_verbose)

--
        Viktor.
Reply | Threaded
Open this post in threaded view
|

Re: milter macro names (potential patch)

Viktor Dukhovni

> On Feb 14, 2017, at 4:06 PM, Matthias Schneider <[hidden email]> wrote:
>
> This patch works like a charm!
> Any chance to get this back into next stable release?

That's a question for Wietse, he may want to solve this in a different way,
or perhaps not at all (arguably your application should be able to deal with
either "{i}" or "i", as the two are semantically equivalent).

The present patch provides you with a stopgap solution, and serves to confirm
the origin of the symptoms.

If you are able to update the milter to deal with either "{i}" or "i", that
would be a good idea.

--
        Viktor.
Reply | Threaded
Open this post in threaded view
|

Re: milter macro names

A. Schulze
In reply to this post by Matthias Schneider


Am 14.02.2017 um 17:54 schrieb Matthias Schneider:
> This broke our milter

Hello,

could you disclose the milter name and version?
Maybe others could avoid some trouble...

Andreas
Reply | Threaded
Open this post in threaded view
|

Re: milter macro names (potential patch)''

Wietse Venema
In reply to this post by Viktor Dukhovni
Viktor Dukhovni:

> On Tue, Feb 14, 2017 at 09:43:25PM +0100, Matthias Schneider wrote:
> > Hi Viktor,
> >
> > i applyed the patch and after connecting to port 25 i'll get:
> >
>
> Yes, sorry, the original patch is buggy, it fails to initialize
> "cname" for already canonical (enclosed in {}) multi-char names.
> Try this one instead:
>
> diff --git a/src/milter/milter.c b/src/milter/milter.c
> index 64836d4..571db7a 100644
> --- a/src/milter/milter.c
> +++ b/src/milter/milter.c
> @@ -333,16 +333,19 @@ static ARGV *milter_macro_lookup(MILTERS *milters, const char *macro_names)
>      VSTRING *canon_buf = vstring_alloc(20);
>      const char *value;
>      const char *name;
> +    const char *cname;
>  
>      while ((name = mystrtok(&cp, CHARS_COMMA_SP)) != 0) {
>   if (msg_verbose)
>      msg_info("%s: \"%s\"", myname, name);
>   if (*name != '{') /* } */
> -    name = STR(vstring_sprintf(canon_buf, "{%s}", name));
> - if ((value = milters->mac_lookup(name, milters->mac_context)) != 0) {
> +    cname = STR(vstring_sprintf(canon_buf, "{%s}", name));
> + else
> +    cname = name;
> + if ((value = milters->mac_lookup(cname, milters->mac_context)) != 0) {
>      if (msg_verbose)
>   msg_info("%s: result \"%s\"", myname, value);
> -    argv_add(argv, name, value, (char *) 0);
> +    argv_add(argv, name[1] == '\0' ? name : cname, value, (char *) 0);
>   } else if (milters->macro_defaults != 0
>       && (value = htable_find(milters->macro_defaults, name)) != 0) {
>      if (msg_verbose)

This breaks milter_macro_defaults lookups, which always require the
form {name}. This would have been found immediately if the library
had tests.

        Wietse
Reply | Threaded
Open this post in threaded view
|

Re: milter macro names (potential patch)''

Matthias Schneider

This breaks milter_macro_defaults lookups, which always require the
form {name}. This would have been found immediately if the library
had tests.

	Wietse

I don't use the new milter_macro_defaults feature yet, so Viktors patch is ok for now.
Hope you can find a good long term solution, the last sendmail version I tested had no curly brackets.
In the meanwhile we will update our internal milter class to support both formats, but the {} may also broke other milter libraries.

Viktor, Wietse, Thank you for support!


Best regards

Matthias Schneider