all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: <emacs@kosowsky.org>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 15866@debbugs.gnu.org, emacs@kosowsky.org
Subject: bug#15866: Gnutls elisp code doesn't properly check for file existence
Date: Tue, 12 Nov 2013 13:12:52 -0500	[thread overview]
Message-ID: <21122.28580.612896.572445@consult.pretender> (raw)
In-Reply-To: <83ob5p1pgd.fsf@gnu.org>

Eli Zaretskii wrote at about 19:48:18 +0200 on Tuesday, November 12, 2013:
 > > Date: Mon, 11 Nov 2013 19:20:08 -0500
 > > From: "" <emacs@kosowsky.org>
 > > 
 > > i]  If the function 'expand-file-name' has an associated magic file
 > >     handler, the function expand-file-name is called to convert it "to
 > >     absolute, and canonicalize it" (quoted from the function
 > >     definition).
 > > 
 > > ii] The test for file-exists-p is then wrapped in a 'let' construct
 > > 	with file-name-handler-alist set to nil. This effectively shuts
 > > 	off magic file handling and ensures that file-exists-p now checks
 > > 	for true OS existence of the now potentially expanded path.
 > > 
 > > iii]The function gnutls-trustfiles is now assured that it will be
 > >     passed an OS-valid path.
 > 
 > Thanks.
 > 
 > As I wrote elsewhere, I agree that gnutls.el should ignore file
 > handlers when it looks for certificate files.
 > 
 > But then _not_ ignoring the expand-file-name handler makes little
 > sense to me: the result could exist as a local file name that has no
 > relation whatsoever to certificates, which will again fail in strange
 > ways inside the GnuTLS library.
 > 
 > So I think we should do ii], but not i].

As I mentioned many times, I would find that an acceptable even if
minimal and non-ideal (for me) solution - provided that it also were
documented in the elisp file and probably also in the
gnutls-trustfiles variable that magic file handling is shut off for
this variable. I am ok with that.

I also think that the following two usability messages should be
added:
1. Warning message (but perhaps not error) triggered if no elements of
   gnutls-trustfiles are valid files
2. Trapping of error if for some reason file-exists-p shows the file
   to exist but for some reason gnutls still can't access it.

In summary, my primary issue was with you declaring the bug summarily
closed when the code clearly was inconsistent in allowing magic file
handling for file-exists-p while not passing on such handling to the
c-routines that actually access the file. Indeed, while one can
disagree with *how* a bug is fixed and to what extent one goes to fix
it, one shouldn't ignore the presence of a bug or sloppy code when
such a simple fix exists.

While it might make little (logical) sense to put ange-ftp or tramp
style paths in gnutls-trustfiles, if one did, they too would cause
this routine to error out. Hence the coding inconsistency is not
limited to cygwin-mount even though the chances of it surfacing
outside of cygwin-mount may be quite small.

So, let's at least agree to the minimal fix for now... I will address
my comments on the pluses/minuses of persistence of cygwin-mount in
response to your other message...


 
 > Btw, I think many Emacs packages don't make sense with remote files,
 > so they should also ignore file handlers.  IOW, this is not specific
 > to gnutls.el.






  reply	other threads:[~2013-11-12 18:12 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-12  0:20 bug#15866: Gnutls elisp code doesn't properly check for file existence emacs
2013-11-12 17:48 ` Eli Zaretskii
2013-11-12 18:12   ` emacs [this message]
2013-11-12 19:41     ` Ted Zlatanov
2013-11-12 19:52   ` Michael Albinus
2013-11-12 20:27     ` Stefan Monnier
2014-12-07 20:17       ` Lars Magne Ingebrigtsen
2014-12-07 21:08         ` Eli Zaretskii
2014-12-07 21:15           ` Lars Magne Ingebrigtsen
2014-12-08  3:32             ` Eli Zaretskii
2014-12-08  7:40               ` Michael Albinus
2014-12-08 18:14               ` Lars Magne Ingebrigtsen
2014-12-08  7:34         ` Michael Albinus
2013-11-12 20:02   ` Stefan Monnier
2013-11-16 23:34     ` Ted Zlatanov
2013-11-17  1:51       ` Stefan Monnier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=21122.28580.612896.572445@consult.pretender \
    --to=emacs@kosowsky.org \
    --cc=15866@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.