Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 22 Apr 2004 15:38:51 +0200
From:      Udo Schweigert <Udo.Schweigert@siemens.com>
To:        Dan Foster <dsf@globalcrossing.net>
Cc:        David O'Brien <obrien@FreeBSD.org>
Subject:   Re: ports/63192: mutt change breaks vim syntax highlighting
Message-ID:  <20040422133850.GA69477@alaska.cert.siemens.de>
In-Reply-To: <20040326042741.GA16229@gblx.net>
References:  <20040324174642.GB18920@anyware12.anyware> <20040324181628.GB6200@dragon.nuxi.com> <20040326042741.GA16229@gblx.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Mar 26, 2004 at 04:27:41 +0000, Dan Foster wrote:
> I'd like to revisit this because I've come across some more information
> that paints things in a different light.
> 
> Specifically, what changed mutt's behavior for the temp file name is
> patch 'patch-mktemp' supplied by the FreeBSD ports maintainer for mutt.
> 
> I just checked the mutt 1.4 and 1.5 source trees (multiple versions),
> and in the unmodified mutt sources, mutt-<version/muttlib.c has this
> function:
> 
> void mutt_mktemp (char *s)
> {
>   snprintf (s, _POSIX_PATH_MAX, "%s/mutt-%s-%d-%d", NONULL (Tempdir),
> NONULL(Hostname), (int) getpid (), Counter++);
>   unlink (s);
}}
> 
> The FreeBSD patch -- which doesn't seem to have come from the mutt
> developers -- has this modification:
> 
> --- muttlib.c.orig      Mon Feb  9 08:25:28 2004
> +++ muttlib.c   Mon Feb  9 08:32:46 2004
> @@ -656,7 +656,8 @@
> 
>  void mutt_mktemp (char *s)
>  {
> -  snprintf (s, _POSIX_PATH_MAX, "%s/mutt-%s-%d-%d", NONULL (Tempdir),
>    NONULL(Hostname), (int) getpid (), Counter++);
> +  snprintf (s, _POSIX_PATH_MAX, "%s/mutt-%s-XXXXXXXX", NONULL(Tempdir),
> NONULL(Hostname));
> +  mktemp (s);
>    unlink (s);
 }}
> 
> My point is that if mutt's behavior changed due to a FreeBSD-specific
> decision, rather than a change made by the mutt developers, then a
> FreeBSD-specific patch (for the syntax highlighting autocommand) should
> also be committed to vim to keep mutt and vim in sync.
> 
> A reasonable person couldn't expect the vim team to accept a patch for
> supporting a change in runtime.vim due to a change in mutt that was not
> committed by the mutt developers and is specific only to FreeBSD.
> 
> This appears to be why vim developers will not commit the patch, and
> they seem to be on solid ground with that position given the reasoning.
> 
> So I would, therefore, like to politely request reconsideration of the
> proposed vim patch for inclusion to the FreeBSD vim port or to have the
> mutt patch-mktemp patch backed out.
> 
> Either change would be sufficient to keep the two in sync and remain
> consistent with policy regarding local changes.

As also written in another mail today (this is for gnats):

Adding the patch was done by another user's request, because the current mutt
code causes problems on open-session machines, when PIDs are reused and thus
mutt sometimes failes to open a tmp-file because it's already opened by
another user. Another benefit is that the usage of mktemp() gives (a little)
more security here.

I also submitted that to the mutt developers mailing list, but it seems it
hasn't been incorporated into the sources yet - and I doubt it will be done
soon, because the whole handling of tmp-files in mutt should be rewritten ...

A possible workaround would be to change the FreeBSD-patch so that the used
tmp-filenames are compatible with vim. That will again lower them security
as one would have to do something like that:

--- muttlib.c.orig	Thu Apr 22 14:43:58 2004
+++ muttlib.c	Thu Apr 22 14:51:01 2004
@@ -657,7 +657,11 @@
 
 void _mutt_mktemp (char *s, const char *src, int line)
 {
-  snprintf (s, _POSIX_PATH_MAX, "%s/mutt-%s-%d-%d", NONULL (Tempdir), NONULL(Hostname), (int) getpid (), Counter++);
+  char t[5];
+  snprintf (t, 5, "-%03d", Counter++);
+  snprintf (s, _POSIX_PATH_MAX-5, "%s/mutt-%s-XXXXXXXX", NONULL(Tempdir), NONULL(Hostname));
+  mktemp (s);
+  strncat(s, t, _POSIX_PATH_MAX);
   dprint (1, (debugfile, "%s:%d: mutt_mktemp returns \"%s\".\n", src, line, s));
   unlink (s);
 }

(Unfortunately there's no mktemps(), and mkstemps() isn't usable here)

It's a bit ugly but it works ;-)


So for me it's up to David O'Brien to decide on either to add a
FreeBSD-specific patch to the vim port(s) or to let me change the patch as
shown above. I won't have a problem with either decission.


Best regards

--
Udo Schweigert, Siemens AG   | Voice      : +49 89 636 42170
CT IC CERT, Siemens CERT     | Fax        : +49 89 636 41166
D-81730 Muenchen / Germany   | email      : udo.schweigert@siemens.com



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040422133850.GA69477>