bug#48902: 28.0.50; Directory names containing apostrophes and backticks cause problems

[Alan Third <alan@idiocy.org>]

[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]

On Tue, Jun 08, 2021 at 09:52:51PM +0200, Mattias Engdegård wrote:
> 8 juni 2021 kl. 21.10 skrev Alan Third <alan@idiocy.org>:
> 
> > I think the attached should solve this.
> 
> Thank you, that would work and I don't mind you pushing that right
> away. We probably should clear up the encodedness of `file` in
> allocInitFromFile: -- as Eli said, the convention is keeping strings
> unencoded until needed by low-level operations and it really makes
> the most sense.

It's not just allocInitFromFile, I'm looking at the other callers of
image_find_image_file and they all call ENCODE_FILE after it too.

The only direct caller of image_find_image_fd that actually uses the
contents of the returned string (svg_load) also encodes the file name.

So I think we could restrict the use of the encoded filename within
image_find_image_fd to *only* when it actually opens the file.

Patch attached. I've tested it here but I only have a couple of images
to try it with.



I've been looking at the other changes I made in
747a923b9a35533f98573ad5b01fccf096195079 and I'm not sure they're
correct. They clearly work now, but most of the time it's probably
simple ASCII which should pass easily.

Before they *all* seem to have assumed the data was UTF8 encoded,
which is surely wrong since most of the time it's coming from Emacs.
It's things like menu item titles.

These are the use cases stringWithLispString was designed for, right?
The only odd one is image filenames because they're explicitly encoded?
-- 
Alan Third

[-- Attachment #2: v2-0001-Fix-image-filename-encoding-issues-bug-48902.patch --]
[-- Type: text/x-diff, Size: 2186 bytes --]

From 48763cfe123955173ad82085125b2f08295daa7d Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Tue, 8 Jun 2021 20:08:34 +0100
Subject: [PATCH v2] Fix image filename encoding issues (bug#48902)

* src/image.c (image_find_image_fd): Don't return an encoded filename
string.
* src/nsfns.m: ([NSString stringWithLispString:]): Clarify usage
comment.
---
 src/image.c | 19 ++++++++-----------
 src/nsfns.m |  3 ++-
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/src/image.c b/src/image.c
index b34dc3e916..d1aaaf8f53 100644
--- a/src/image.c
+++ b/src/image.c
@@ -3153,19 +3153,16 @@ image_find_image_fd (Lisp_Object file, int *pfd)
   /* Try to find FILE in data-directory/images, then x-bitmap-file-path.  */
   fd = openp (search_path, file, Qnil, &file_found,
 	      pfd ? Qt : make_fixnum (R_OK), false, false);
-  if (fd >= 0 || fd == -2)
+  if (fd == -2)
     {
-      file_found = ENCODE_FILE (file_found);
-      if (fd == -2)
-	{
-	  /* The file exists locally, but has a file name handler.
-	     (This happens, e.g., under Auto Image File Mode.)
-	     'openp' didn't open the file, so we should, because the
-	     caller expects that.  */
-	  fd = emacs_open (SSDATA (file_found), O_RDONLY, 0);
-	}
+      /* The file exists locally, but has a file name handler.
+	 (This happens, e.g., under Auto Image File Mode.)
+	 'openp' didn't open the file, so we should, because the
+	 caller expects that.  */
+      Lisp_Object encoded_name = ENCODE_FILE (file_found);
+      fd = emacs_open (SSDATA (encoded_name), O_RDONLY, 0);
     }
-  else	/* fd < 0, but not -2 */
+  else if (fd < 0)
     return Qnil;
   if (pfd)
     *pfd = fd;
diff --git a/src/nsfns.m b/src/nsfns.m
index d14f7b51ea..98801d8526 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -3024,7 +3024,8 @@ - (NSString *)panel: (id)sender userEnteredFilename: (NSString *)filename
 }
 
 @implementation NSString (EmacsString)
-/* Make an NSString from a Lisp string.  */
+/* Make an NSString from a Lisp string.  STRING must not be in an
+   encoded form (e.g. UTF-8).  */
 + (NSString *)stringWithLispString:(Lisp_Object)string
 {
   /* Shortcut for the common case.  */
-- 
2.30.2