GNU bug report logs - #48902
28.0.50; Directory names containing apostrophes and backticks cause problems

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 48902 in the body.
You can then email your comments to 48902 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Rudolf Adamkovič <salutis <at> me.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; Directory names containing apostrophes and backticks cause
 problems
Date: Mon, 07 Jun 2021 15:32:10 +0200

This is my first Emacs bug report.

DESCRIPTION:

I use EMMS to listen to music, and recently, I have noticed that the
EMMS Browser does not show covers for some of my albums. Today, I
decided to investigate the problem, and I found the following.

REPRODUCTION STEPS:

1. Create a directory called "Dubmood - C’etait Mieux En Rda (DATA001)".
2. Copy a picture, say "cover.png" into the newly created directory.
3. Launch "emacs -Q".
3. Open the newly created directory in Dired.
4. Press RET on the picture.

EXPECTED RESULTS:

Emacs shows the picture.

ACTUAL RESULTS:

Emacs shows an empty window.

NOTES:

The same problem applies to a folder called "Ultrasyd - L'Épidemie
Dansante". It seems that both an apostrophe (') and a backtick (`)
cause problems in both EMMS and Dired.

-- Rudy

In GNU Emacs 28.0.50 (build 2, x86_64-apple-darwin20.3.0, NS appkit-2022.30 Version 11.2.3 (Build 20D91))
 of 2021-06-07 built on Workstation.local
Repository revision: 82ccc3afcf9ed1f8b22ed5634e788879cd1af279
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2022
System Description:  macOS 11.2.3

Configured using:
 'configure --with-ns --with-native-compilation'

Configured features:
ACL DBUS GIF GLIB GMP GNUTLS JPEG JSON LCMS2 LIBXML2 MODULES NATIVE_COMP
NOTIFY KQUEUE NS PDUMPER PNG RSVG THREADS TIFF TOOLKIT_SCROLL_BARS XIM
ZLIB

Important settings:
  value of $LANG: en_SK.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  gnus-undo-mode: t
  shell-dirtrack-mode: t
  dumb-jump-mode: t
  global-hl-todo-mode: t
  show-paren-mode: t
  global-flycheck-mode: t
  recentf-mode: t
  counsel-projectile-mode: t
  projectile-mode: t
  global-subword-mode: t
  subword-mode: t
  all-the-icons-ivy-rich-mode: t
  ivy-prescient-mode: t
  prescient-persist-mode: t
  ivy-rich-project-root-cache-mode: t
  ivy-rich-mode: t
  ivy-mode: t
  guru-global-mode: t
  guru-mode: t
  which-key-mode: t
  save-place-mode: t
  global-auto-revert-mode: t
  delete-selection-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  global-visual-line-mode: t
  visual-line-mode: t
  transient-mark-mode: t

Load-path shadows:
/Users/salutis/.emacs.d/elpa/transient-20210530.2252/transient hides /Users/salutis/Repositories/emacs/nextstep/Emacs.app/Contents/Resources/lisp/transient

Features:
(shadow emacsbug sendmail smiley gnus-cite mail-extr gnus-async
gnus-bcklg sort gnus-ml mm-archive gnutls network-stream url-http url-gw
nsm url-cache url-auth qp nnrss mm-url nndraft nnmh nnmaildir nnfolder
nnnil gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg
gnus-art mm-uu mml2015 mm-view mml-smime smime dig nntp gnus-cache
gnus-sum shr kinsoku svg dom gnus-group gnus-undo gnus-start gnus-dbus
dbus xml gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo gnus-spec
gnus-int gnus-range message rmc puny rfc822 mml mml-sec epa derived epg
epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader gnus-win em-unix em-term em-script em-prompt em-ls
em-hist em-pred em-glob em-dirs esh-var em-cmpl em-basic em-banner
em-alias esh-mode bookmark pp writegood-mode two-column password-store
auth-source-pass with-editor vterm face-remap vterm-module term/xterm
xterm term disp-table ehelp eshell esh-cmd esh-ext esh-opt esh-proc
esh-io esh-arg esh-module esh-groups esh-util server amx vc-git
diff-mode vc-dispatcher ffap tramp tramp-loaddefs trampver
tramp-integration files-x tramp-compat shell parse-time iso8601 ls-lisp
pcase flyspell ispell dumb-jump popup hl-todo gnus nnheader gnus-util
rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums mail-utils mm-util
mail-prsvr cus-load paren flycheck recentf tree-widget wid-edit
counsel-projectile ag vc-svn find-dired s dash ripgrep projectile grep
ibuf-ext ibuffer ibuffer-loaddefs thingatpt cap-words superword subword
all-the-icons-ivy-rich all-the-icons all-the-icons-faces data-material
data-weathericons data-octicons data-fileicons data-faicons
data-alltheicons ivy-prescient prescient counsel xdg xref project dired
dired-loaddefs compile text-property-search swiper ivy-rich ivy flx
ivy-faces ivy-overlay colir color guru-mode which-key
modus-vivendi-theme modus-operandi-theme modus-themes comp comp-cstr
warnings rx saveplace autorevert filenotify delsel exec-path-from-shell
diminish use-package-ensure-system-package system-packages use-package
use-package-ensure use-package-delight use-package-diminish
use-package-bind-key bind-key use-package-core finder-inf cl-extra
help-mode org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro
org-footnote org-src ob-comint org-pcomplete pcomplete comint ansi-color
ring org-list org-faces org-entities time-date noutline outline
easy-mmode org-version ob-emacs-lisp ob-core ob-eval org-table ol
org-keys org-compat advice org-macs org-loaddefs format-spec find-func
cal-menu calendar cal-loaddefs tex-site edmacro kmacro info package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib iso-transl tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/ns-win ns-win
ucs-normalize mule-util term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
kqueue cocoa ns lcms2 multi-tty make-network-process native-compile
emacs)

Memory information:
((conses 16 862734 56170)
 (symbols 48 42507 4)
 (strings 32 248048 23357)
 (string-bytes 1 10036170)
 (vectors 16 87234)
 (vector-slots 8 2207490 61351)
 (floats 8 1197 526)
 (intervals 56 735 540)
 (buffers 992 25))

Message #8 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: 48902 <at> debbugs.gnu.org
Cc: Rudolf Adamkovič <salutis <at> me.com>
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Mon, 07 Jun 2021 16:08:31 +0200

Rudolf Adamkovič via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> gnu.org> writes:

> 1. Create a directory called "Dubmood - C’etait Mieux En Rda (DATA001)".
> 2. Copy a picture, say "cover.png" into the newly created directory.
> 3. Launch "emacs -Q".
> 3. Open the newly created directory in Dired.
> 4. Press RET on the picture.
>
> EXPECTED RESULTS:
>
> Emacs shows the picture.
>
> ACTUAL RESULTS:
>
> Emacs shows an empty window.

I'm unable to reproduce this problem on Debian/bullseye...

> In GNU Emacs 28.0.50 (build 2, x86_64-apple-darwin20.3.0, NS appkit-2022.30 Version 11.2.3 (Build 20D91))
>  of 2021-06-07 built on Workstation.local

... so perhaps it's only a problem for the Macos build?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #11 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: 48902 <at> debbugs.gnu.org
Cc: Alan Third <alan <at> idiocy.org>,
 Rudolf Adamkovič <salutis <at> me.com>
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Mon, 07 Jun 2021 16:24:44 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> ... so perhaps it's only a problem for the Macos build?

Yup; the current trunk isn't able to view images under Macos from dired
if the path contains a non-ASCII char.

Test case:

larsi <at> emkay trunk % mkdir /tmp/fóo 
larsi <at> emkay trunk % cp etc/images/gnus/gnus.png /tmp/fóo/
larsi <at> emkay trunk % ./src/emacs /tmp/fóo/

And then RET the file to test.  I then get an error message about
failed output type 'public.tiff' on the terminal.

I've added Alan to the CCs.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #14 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 48902 <at> debbugs.gnu.org, alan <at> idiocy.org, salutis <at> me.com
Subject: Re: bug#48902: 28.0.50;
 Directory names containing apostrophes and backticks cause problems
Date: Mon, 07 Jun 2021 17:36:37 +0300

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Mon, 07 Jun 2021 16:24:44 +0200
> Cc: Alan Third <alan <at> idiocy.org>,
>  Rudolf Adamkovič <salutis <at> me.com>
> 
> larsi <at> emkay trunk % mkdir /tmp/fóo 
> larsi <at> emkay trunk % cp etc/images/gnus/gnus.png /tmp/fóo/
> larsi <at> emkay trunk % ./src/emacs /tmp/fóo/
> 
> And then RET the file to test.  I then get an error message about
> failed output type 'public.tiff' on the terminal.

What happens if you type

  larsi <at> emkay trunk % ./src/emacs /tmp/fóo/gnus.png

does it show the PNG image then?

And what are the values of file-name-coding-system and
default-file-name-coding-system?

Message #17 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 48902 <at> debbugs.gnu.org, salutis <at> me.com
Subject: Re: bug#48902: 28.0.50;
 Directory names containing apostrophes and backticks cause problems
Date: Mon, 07 Jun 2021 17:15:14 +0300

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Mon, 07 Jun 2021 16:08:31 +0200
> Cc: Rudolf Adamkovič <salutis <at> me.com>
> 
> I'm unable to reproduce this problem on Debian/bullseye...
> 
> > In GNU Emacs 28.0.50 (build 2, x86_64-apple-darwin20.3.0, NS appkit-2022.30 Version 11.2.3 (Build 20D91))
> >  of 2021-06-07 built on Workstation.local
> 
> ... so perhaps it's only a problem for the Macos build?

I suspect it has to do with the special way macOS encodes non-ASCII
file names.

Message #20 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Rudolf Adamkovič <salutis <at> me.com>
Cc: 48902 <at> debbugs.gnu.org
Subject: Re: bug#48902: 28.0.50;
 Directory names containing apostrophes and backticks cause problems
Date: Mon, 07 Jun 2021 17:13:01 +0300

> Date: Mon, 07 Jun 2021 15:32:10 +0200
> From:  Rudolf Adamkovič via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> 1. Create a directory called "Dubmood - C’etait Mieux En Rda (DATA001)".
> 2. Copy a picture, say "cover.png" into the newly created directory.
> 3. Launch "emacs -Q".
> 3. Open the newly created directory in Dired.
> 4. Press RET on the picture.
> 
> EXPECTED RESULTS:
> 
> Emacs shows the picture.
> 
> ACTUAL RESULTS:
> 
> Emacs shows an empty window.
> 
> NOTES:
> 
> The same problem applies to a folder called "Ultrasyd - L'Épidemie
> Dansante". It seems that both an apostrophe (') and a backtick (`)
> cause problems in both EMMS and Dired.

When you visit that directory in Dired, what is the value of
buffer-file-coding-system in the Dired buffer?

Also, you mention backtick, but there's no such character in the
example you show.

Message #23 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: naofumi <at> yasufuku.dev <naofumi <at> yasufuku.dev>
To: "Eli Zaretskii" <eliz <at> gnu.org>
Cc: 48902 <at> debbugs.gnu.org, Rudolf Adamkovič <salutis <at> me.com>
Subject: Re: bug#48902: 28.0.50;
 Directory names containing apostrophes and backticks cause problems
Date: Tue, 08 Jun 2021 12:39:19 +0200

[Message part 1 (text/plain, inline)]

On Monday, June 07, 2021 16:15 CEST, Eli Zaretskii <eliz <at> gnu.org> wrote:
> > From: Lars Ingebrigtsen <larsi <at> gnus.org>
> > ... so perhaps it's only a problem for the Macos build?
>
> I suspect it has to do with the special way macOS encodes non-ASCII
> file names.
>

I found the same issue with images which has Japanese filename on macOS.
Message buffer says:
------------------------------------------------------------------------
Unable to load image (image :type png :file /Users/naofumi/_git/git.sv.gnu.org/emacs_png/いーまっくす.png :scale 1 :max-width 480 :max-height 781 :format nil :transform-smoothing t) [5 times]
------------------------------------------------------------------------

At least, attached small patch could fix this.

attachments:
0001-Fix-to-show-images-with-non-ascii-filename-on-macOS.patch
ns_load_image-error-with-non-ascii-filename.png
revert-filename-NSString-in-nsimage.png
emacs_png.tar.gz

This [EmacsImage allocInitFromFile:] change was introduced by the
following commit:
------------------------------------------------------------------------
commit 747a923b9a35533f98573ad5b01fccf096195079
Author: Alan Third <alan <at> idiocy.org>
Date:   Tue Dec 22 23:28:25 2020 +0000

    Use new NSString lisp methods

    * src/nsfont.m (ns_otf_to_script):
    (ns_registry_to_script):
    (ns_get_req_script): Use NSString conversion methods.
    * src/nsimage.m ([EmacsImage allocInitFromFile:]): Use NSString
    conversion methods.
    * src/nsmenu.m (ns_menu_show): Use NSString conversion methods.    * src/nsselect.m (symbol_to_nsstring):
    (ns_string_to_pasteboard_internal): Use NSString conversion methods.
    * src/nsterm.m (ns_term_init):
    ([EmacsView initFrameFromEmacs:]): Use NSString conversion methods.    * src/nsxwidget.m (nsxwidget_webkit_uri):
    (nsxwidget_webkit_title):
    (js_to_lisp): Use NSString conversion methods.
    (build_string_with_nsstr): Functionality replaced by NSString    extensions.

------------------------------------------------------------------------

Regards,
--Naofumi

[emacs_png.tar.gz (application/x-gzip, attachment)]

[ns_load_image-error-with-non-ascii-filename.png (image/png, inline)]

[0001-Fix-to-show-images-with-non-ascii-filename-on-macOS.patch (application/octet-stream, attachment)]

[revert-filename-NSString-in-nsimage.png (image/png, inline)]

Message #26 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: naofumi <at> yasufuku.dev
Cc: 48902 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Rudolf Adamkovič <salutis <at> me.com>,
 Alan Third <alan <at> idiocy.org>
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 08 Jun 2021 13:57:01 +0200

naofumi <at> yasufuku.dev <naofumi <at> yasufuku.dev> writes:

> diff --git a/src/nsimage.m b/src/nsimage.m
> index fa81a41a51..8c7a3d9a09 100644
> --- a/src/nsimage.m
> +++ b/src/nsimage.m
> @@ -262,7 +262,7 @@ + (instancetype)allocInitFromFile: (Lisp_Object)file
>    found = ENCODE_FILE (found);
>  
>    image = [[EmacsImage alloc] initByReferencingFile:
> -                     [NSString stringWithLispString: found]];
> +                     [NSString stringWithUTF8String: SSDATA (found)]];

Hm...  I'm not very familiar at all with the Objective C code here...
but shouldn't "found" here be a Lisp string so that stringWithLispString
would do the right thing?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #29 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 48902 <at> debbugs.gnu.org, Rudolf Adamkovič <salutis <at> me.com>,
 naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 8 Jun 2021 13:12:56 +0100

On Tue, Jun 08, 2021 at 01:57:01PM +0200, Lars Ingebrigtsen wrote:
> naofumi <at> yasufuku.dev <naofumi <at> yasufuku.dev> writes:
> 
> > diff --git a/src/nsimage.m b/src/nsimage.m
> > index fa81a41a51..8c7a3d9a09 100644
> > --- a/src/nsimage.m
> > +++ b/src/nsimage.m
> > @@ -262,7 +262,7 @@ + (instancetype)allocInitFromFile: (Lisp_Object)file
> >    found = ENCODE_FILE (found);
> >  
> >    image = [[EmacsImage alloc] initByReferencingFile:
> > -                     [NSString stringWithLispString: found]];
> > +                     [NSString stringWithUTF8String: SSDATA (found)]];
> 
> Hm...  I'm not very familiar at all with the Objective C code here...
> but shouldn't "found" here be a Lisp string so that stringWithLispString
> would do the right thing?

It's always possible that stringWithLispString isn't doing the right
thing. It's implemented at nsfns.m:3026. I know almost nothing about
UTF8/UTF16 so while it looks like it's doing the right thing to me, I
could be entirely wrong.
-- 
Alan Third

Message #32 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Alan Third <alan <at> idiocy.org>
Cc: 48902 <at> debbugs.gnu.org,
 Mattias Engdegård <mattiase <at> acm.org>,
 Rudolf Adamkovič <salutis <at> me.com>,
 naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 08 Jun 2021 14:14:53 +0200

Alan Third <alan <at> idiocy.org> writes:

>> >    image = [[EmacsImage alloc] initByReferencingFile:
>> > -                     [NSString stringWithLispString: found]];
>> > +                     [NSString stringWithUTF8String: SSDATA (found)]];
>> 
>> Hm...  I'm not very familiar at all with the Objective C code here...
>> but shouldn't "found" here be a Lisp string so that stringWithLispString
>> would do the right thing?
>
> It's always possible that stringWithLispString isn't doing the right
> thing. It's implemented at nsfns.m:3026. I know almost nothing about
> UTF8/UTF16 so while it looks like it's doing the right thing to me, I
> could be entirely wrong.

Right -- and that was written by Mattias, so I've added him to the CCs.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #35 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Third <alan <at> idiocy.org>
Cc: 48902 <at> debbugs.gnu.org, larsi <at> gnus.org, salutis <at> me.com, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50;
 Directory names containing apostrophes and backticks cause problems
Date: Tue, 08 Jun 2021 15:37:51 +0300

> Date: Tue, 8 Jun 2021 13:12:56 +0100
> From: Alan Third <alan <at> idiocy.org>
> Cc: 48902 <at> debbugs.gnu.org, Rudolf Adamkovič <salutis <at> me.com>,
>  naofumi <at> yasufuku.dev
> 
> On Tue, Jun 08, 2021 at 01:57:01PM +0200, Lars Ingebrigtsen wrote:
> > naofumi <at> yasufuku.dev <naofumi <at> yasufuku.dev> writes:
> > 
> > > diff --git a/src/nsimage.m b/src/nsimage.m
> > > index fa81a41a51..8c7a3d9a09 100644
> > > --- a/src/nsimage.m
> > > +++ b/src/nsimage.m
> > > @@ -262,7 +262,7 @@ + (instancetype)allocInitFromFile: (Lisp_Object)file
> > >    found = ENCODE_FILE (found);
> > >  
> > >    image = [[EmacsImage alloc] initByReferencingFile:
> > > -                     [NSString stringWithLispString: found]];
> > > +                     [NSString stringWithUTF8String: SSDATA (found)]];
> > 
> > Hm...  I'm not very familiar at all with the Objective C code here...
> > but shouldn't "found" here be a Lisp string so that stringWithLispString
> > would do the right thing?
> 
> It's always possible that stringWithLispString isn't doing the right
> thing. It's implemented at nsfns.m:3026. I know almost nothing about
> UTF8/UTF16 so while it looks like it's doing the right thing to me, I
> could be entirely wrong.

It looks like stringWithLispString encodes into UTF-16?  But file
names on macOS should be encoded in UTF-8, and in fact
allocInitFromFile already does TRT when it calls ENCODE_FILE, just
before stringWithLispString is called.  So I think the patch is
correct.

(UTF-16 encoding on macOS is for ENCODE_SYSTEM, right?)

Message #38 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 48902 <at> debbugs.gnu.org, larsi <at> gnus.org, salutis <at> me.com, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 8 Jun 2021 14:00:17 +0100

On Tue, Jun 08, 2021 at 03:37:51PM +0300, Eli Zaretskii wrote:
> > Date: Tue, 8 Jun 2021 13:12:56 +0100
> > From: Alan Third <alan <at> idiocy.org>
> > Cc: 48902 <at> debbugs.gnu.org, Rudolf Adamkovič <salutis <at> me.com>,
> >  naofumi <at> yasufuku.dev
> > 
> > On Tue, Jun 08, 2021 at 01:57:01PM +0200, Lars Ingebrigtsen wrote:
> > > naofumi <at> yasufuku.dev <naofumi <at> yasufuku.dev> writes:
> > > 
> > > > diff --git a/src/nsimage.m b/src/nsimage.m
> > > > index fa81a41a51..8c7a3d9a09 100644
> > > > --- a/src/nsimage.m
> > > > +++ b/src/nsimage.m
> > > > @@ -262,7 +262,7 @@ + (instancetype)allocInitFromFile: (Lisp_Object)file
> > > >    found = ENCODE_FILE (found);
> > > >  
> > > >    image = [[EmacsImage alloc] initByReferencingFile:
> > > > -                     [NSString stringWithLispString: found]];
> > > > +                     [NSString stringWithUTF8String: SSDATA (found)]];
> > > 
> > > Hm...  I'm not very familiar at all with the Objective C code here...
> > > but shouldn't "found" here be a Lisp string so that stringWithLispString
> > > would do the right thing?
> > 
> > It's always possible that stringWithLispString isn't doing the right
> > thing. It's implemented at nsfns.m:3026. I know almost nothing about
> > UTF8/UTF16 so while it looks like it's doing the right thing to me, I
> > could be entirely wrong.
> 
> It looks like stringWithLispString encodes into UTF-16?  But file
> names on macOS should be encoded in UTF-8, and in fact
> allocInitFromFile already does TRT when it calls ENCODE_FILE, just
> before stringWithLispString is called.  So I think the patch is
> correct.
> 
> (UTF-16 encoding on macOS is for ENCODE_SYSTEM, right?)

I think you're right. But confusingly initByReferencingFile takes an
NSString which is a UTF-16 format string, so if I remove all the calls
to ENCODE_FILE, stringWithLispString works fine.

I guess we just need to make a note that stringWithLispString cannot
handle UTF-8 encoded filenames, unless someone has a smarter solution.

-- 
Alan Third

Message #41 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Third <alan <at> idiocy.org>
Cc: 48902 <at> debbugs.gnu.org, larsi <at> gnus.org, salutis <at> me.com, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 08 Jun 2021 17:02:25 +0300

> Date: Tue, 8 Jun 2021 14:00:17 +0100
> From: Alan Third <alan <at> idiocy.org>
> Cc: larsi <at> gnus.org, naofumi <at> yasufuku.dev, 48902 <at> debbugs.gnu.org,
> 	salutis <at> me.com
> 
> > It looks like stringWithLispString encodes into UTF-16?  But file
> > names on macOS should be encoded in UTF-8, and in fact
> > allocInitFromFile already does TRT when it calls ENCODE_FILE, just
> > before stringWithLispString is called.  So I think the patch is
> > correct.
> > 
> > (UTF-16 encoding on macOS is for ENCODE_SYSTEM, right?)
> 
> I think you're right. But confusingly initByReferencingFile takes an
> NSString which is a UTF-16 format string, so if I remove all the calls
> to ENCODE_FILE, stringWithLispString works fine.
> 
> I guess we just need to make a note that stringWithLispString cannot
> handle UTF-8 encoded filenames, unless someone has a smarter solution.

If you do need a UTF-16 encoded string, then instead of ENCODE_FILE
you can call code_convert_string_norecord with Qutf_16.  There's no
need to invent or use a private UTF-16 encoder there, and you also get
rid of an unnecessary extra UTF-8 encoding as a bonus.

Message #44 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 48902 <at> debbugs.gnu.org, larsi <at> gnus.org, salutis <at> me.com, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 8 Jun 2021 17:19:44 +0100

On Tue, Jun 08, 2021 at 05:02:25PM +0300, Eli Zaretskii wrote:
> > Date: Tue, 8 Jun 2021 14:00:17 +0100
> > From: Alan Third <alan <at> idiocy.org>
> > Cc: larsi <at> gnus.org, naofumi <at> yasufuku.dev, 48902 <at> debbugs.gnu.org,
> > 	salutis <at> me.com
> > 
> > > It looks like stringWithLispString encodes into UTF-16?  But file
> > > names on macOS should be encoded in UTF-8, and in fact
> > > allocInitFromFile already does TRT when it calls ENCODE_FILE, just
> > > before stringWithLispString is called.  So I think the patch is
> > > correct.
> > > 
> > > (UTF-16 encoding on macOS is for ENCODE_SYSTEM, right?)
> > 
> > I think you're right. But confusingly initByReferencingFile takes an
> > NSString which is a UTF-16 format string, so if I remove all the calls
> > to ENCODE_FILE, stringWithLispString works fine.
> > 
> > I guess we just need to make a note that stringWithLispString cannot
> > handle UTF-8 encoded filenames, unless someone has a smarter solution.
> 
> If you do need a UTF-16 encoded string, then instead of ENCODE_FILE
> you can call code_convert_string_norecord with Qutf_16.  There's no
> need to invent or use a private UTF-16 encoder there, and you also get
> rid of an unnecessary extra UTF-8 encoding as a bonus.

In this case the call to ENCODE_FILE in allocInitFromFile is actually
redundant because image_find_image_fd already calls ENCODE_FILE on the
filename before passing it back. So we get a UTF-8 string no matter
what.

NSString can read in almost anything, and Mattias extended it to read
in multibyte (and ascii) lisp strings, so we don't need a UTF-16 input
specifically. It would probably be nice if NSString was also able to
recognise that a lisp string is UTF-8 and handle that itself, but I
don't think that's really possible, unless we make the assumption that
any unibyte string it's passed will already be ascii or UTF-8.

I don't know if that's a reasonable assumption.

-- 
Alan Third

Message #47 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 48902 <at> debbugs.gnu.org, Alan Third <alan <at> idiocy.org>,
 Rudolf Adamkovič <salutis <at> me.com>, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 8 Jun 2021 19:45:22 +0200

8 juni 2021 kl. 14.14 skrev Lars Ingebrigtsen <larsi <at> gnus.org>:

>> It's always possible that stringWithLispString isn't doing the right
>> thing. It's implemented at nsfns.m:3026. I know almost nothing about
>> UTF8/UTF16 so while it looks like it's doing the right thing to me, I
>> could be entirely wrong.
> 
> Right -- and that was written by Mattias, so I've added him to the CCs.

Thank you, but stringWithLispString: is actually fine, unless you count its inability to produce useful results from wrong input!

The image code seems to be quite confused with respect to whether the file names being passed around are in encoded form. Until recently it seems to have worked by pure chance since as long as the file name encoding is UTF-8 and the low-level code accesses the raw string data we do get the same result, but at least since 747a923b9a35 that's no longer the case.

Concretely, we have:

1. image_find_image_file probably expects its `file` argument to be non-encoded, but the string it returns is always encoded.

2. native_image_load calls image_find_image_file and passes its return value to ns_load_image.

3. ns_load_image calls [EmacsImage allocInitFromFile:] with its file argument.

4. [EmacsImage allocInitFromFile: file] can apparently be called with both a non-encoded or an encoded `file` argument (clearly not ideal), and it does:

  found = image_find_image_file (file);
  // This is dubious when `file` is already encoded.

  found = ENCODE_FILE (found);
  // This is completely useless since `found` is already encoded! Apparently ENCODE_FILE is idempotent, at least on macOS...

  [NSString stringWithLispString: found]
  // This produces nonsense as `found` is a string of raw bytes, so any Unicode will be converted to stretches of U+FFFD REPLACEMENT CHAR.
  [NSString stringWithLispString: file]
  // Same problem again, with a different variable.

The quick fix of reverting to stringWithUTF8String: will work, but the real problem is that we have no control of the encodedness of lisp strings being passed around. Comments would help, and I'd even go as far to suggest

typedef struct { Lisp_Object string; } encoded_file_name_t;

with the appropriate constructors and accessors, to get C's static type checking to work for us.

Message #50 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Third <alan <at> idiocy.org>
Cc: 48902 <at> debbugs.gnu.org, larsi <at> gnus.org, salutis <at> me.com, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 08 Jun 2021 21:09:51 +0300

> Date: Tue, 8 Jun 2021 17:19:44 +0100
> From: Alan Third <alan <at> idiocy.org>
> Cc: larsi <at> gnus.org, naofumi <at> yasufuku.dev, 48902 <at> debbugs.gnu.org,
> 	salutis <at> me.com
> 
> In this case the call to ENCODE_FILE in allocInitFromFile is actually
> redundant because image_find_image_fd already calls ENCODE_FILE on the
> filename before passing it back. So we get a UTF-8 string no matter
> what.

Then why was the code re-encoding t in UTF-16?  A bug?

> NSString can read in almost anything, and Mattias extended it to read
> in multibyte (and ascii) lisp strings, so we don't need a UTF-16 input
> specifically. It would probably be nice if NSString was also able to
> recognise that a lisp string is UTF-8 and handle that itself, but I
> don't think that's really possible, unless we make the assumption that
> any unibyte string it's passed will already be ascii or UTF-8.
> 
> I don't know if that's a reasonable assumption.

Any file name passed through ENCODE_FILE should be in UTF-8 on macOS,
as I understand that's how the macOS filesystems work.  Am I mistaken?
Can the value of default-file-name-coding-system on macOS be anything
other than UTF-8?

Message #53 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Alan Third <alan <at> idiocy.org>
Cc: 48902 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Rudolf Adamkovič <salutis <at> me.com>,
 Eli Zaretskii <eliz <at> gnu.org>, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 8 Jun 2021 20:17:57 +0200

[Replying to a couple of previous messages]

> I guess we just need to make a note that stringWithLispString cannot
> handle UTF-8 encoded filenames, unless someone has a smarter solution.

This is not restricted to file names but yes, we should definitely clarify that it expects Unicode (or ASCII) strings as input, since raw bytes are interpreted as, well, raw bytes.

> NSString can read in almost anything, and Mattias extended it to read
> in multibyte (and ascii) lisp strings, so we don't need a UTF-16 input
> specifically. It would probably be nice if NSString was also able to
> recognise that a lisp string is UTF-8 and handle that itself, but I
> don't think that's really possible, unless we make the assumption that
> any unibyte string it's passed will already be ascii or UTF-8.
> 
> I don't know if that's a reasonable assumption.

No, I don't think it's reasonable either -- we should not put dwimmery into our string conversion logic just because we are too sloppy to document whether an argument or return value is encoded or not. stringWithLispString: appears to work as designed.

Message #56 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 48902 <at> debbugs.gnu.org, larsi <at> gnus.org, salutis <at> me.com, alan <at> idiocy.org,
 naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50;
 Directory names containing apostrophes and backticks cause problems
Date: Tue, 08 Jun 2021 21:18:57 +0300

> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Tue, 8 Jun 2021 19:45:22 +0200
> Cc: 48902 <at> debbugs.gnu.org, Alan Third <alan <at> idiocy.org>,
>  Rudolf Adamkovič <salutis <at> me.com>, naofumi <at> yasufuku.dev
> 
> The quick fix of reverting to stringWithUTF8String: will work, but the real problem is that we have no control of the encodedness of lisp strings being passed around.

The usual technique in these cases is to keep Lisp strings unencoded,
encode them when passing to the low-level C functions, and pass to
those functions only the pointer to the string's data.

In those rare cases when you really need to pass a Lisp string that is
an encoded file name, call the argument "encoded_file" or somesuch.
But these cases should be rare exceptions.

Message #59 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 48902 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Rudolf Adamkovič <salutis <at> me.com>, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 8 Jun 2021 20:10:04 +0100

[Message part 1 (text/plain, inline)]

On Tue, Jun 08, 2021 at 07:45:22PM +0200, Mattias Engdegård wrote:
> 8 juni 2021 kl. 14.14 skrev Lars Ingebrigtsen <larsi <at> gnus.org>:
> 
> >> It's always possible that stringWithLispString isn't doing the right
> >> thing. It's implemented at nsfns.m:3026. I know almost nothing about
> >> UTF8/UTF16 so while it looks like it's doing the right thing to me, I
> >> could be entirely wrong.
> > 
> > Right -- and that was written by Mattias, so I've added him to the CCs.
> 
> Thank you, but stringWithLispString: is actually fine, unless you
> count its inability to produce useful results from wrong input!

In my defence it wasn't entirely clear to me that a lisp string
returned from ENCODE_FILE was incompatible with stringWithLispString. ;)

> The image code seems to be quite confused with respect to whether
> the file names being passed around are in encoded form. Until
> recently it seems to have worked by pure chance since as long as the
> file name encoding is UTF-8 and the low-level code accesses the raw
> string data we do get the same result, but at least since
> 747a923b9a35 that's no longer the case.

Hmm, and as you point out we use "file" further down and it may or may
not be encoded, but will probably have the same contents as found,
which we know is encoded. Plus it's setting the "name" field in the
image, which we probably want to keep as uniform as possible for
caching purposes but is otherwise irrelevant.

I think the attached should solve this.
-- 
Alan Third

[0001-Fix-image-filename-encoding-issues-in-NS-port-bug-48.patch (text/x-diff, attachment)]

Message #62 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: naofumi <at> yasufuku.dev <naofumi <at> yasufuku.dev>
To: "Eli Zaretskii" <eliz <at> gnu.org>
Cc: 48902 <at> debbugs.gnu.org,
 Mattias Engdegård <mattiase <at> acm.org>, larsi <at> gnus.org,
 salutis <at> me.com, alan <at> idiocy.org
Subject: Re: bug#48902: 28.0.50;
 Directory names containing apostrophes and backticks cause problems
Date: Tue, 08 Jun 2021 21:13:13 +0200

[Message part 1 (text/plain, inline)]

On Tuesday, June 08, 2021 20:18 CEST, Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Mattias Engdegård <mattiase <at> acm.org>
> > Date: Tue, 8 Jun 2021 19:45:22 +0200
> > Cc: 48902 <at> debbugs.gnu.org, Alan Third <alan <at> idiocy.org>,
> >  Rudolf Adamkovič <salutis <at> me.com>, naofumi <at> yasufuku.dev
> >
> > The quick fix of reverting to stringWithUTF8String: will work, but the real problem is that we have no control of the encodedness of lisp strings being passed around.
>
> The usual technique in these cases is to keep Lisp strings unencoded,
> encode them when passing to the low-level C functions, and pass to
> those functions only the pointer to the string's data.
>
> In those rare cases when you really need to pass a Lisp string that is
> an encoded file name, call the argument "encoded_file" or somesuch.
> But these cases should be rare exceptions.
>
>

I agree that [NSString stringWithLispString:] is working as expected,
and it is not the real problem.

For example, another patch using STRING_SET_MULTIBYTE() is here.
This looks still just a quick fix and bit weired, though..

attachments:
0001-Fix-to-show-images-with-non-ascii-filename-STRING_SET_MULTIBYTE.patch
image-non-ascii-filename-STRING_SET_MULTIBYTE-macos.png
image-non-ascii-filename-STRING_SET_MULTIBYTE-linux.png

On the other hand, I cannot find out that non-UTF-8 filename coding is
really needed on macOS.  It might be over-engineered thing, and just an overhead.

Regards,
--Naofumi

[image-non-ascii-filename-STRING_SET_MULTIBYTE-macos.png (image/png, inline)]

[0001-Fix-to-show-images-with-non-ascii-filename-STRING_SET_MULTIBYTE.patch (application/octet-stream, attachment)]

[image-non-ascii-filename-STRING_SET_MULTIBYTE-linux.png (image/png, inline)]

Message #65 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 48902 <at> debbugs.gnu.org, larsi <at> gnus.org, salutis <at> me.com, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 8 Jun 2021 20:24:26 +0100

On Tue, Jun 08, 2021 at 09:09:51PM +0300, Eli Zaretskii wrote:
> > Date: Tue, 8 Jun 2021 17:19:44 +0100
> > From: Alan Third <alan <at> idiocy.org>
> > Cc: larsi <at> gnus.org, naofumi <at> yasufuku.dev, 48902 <at> debbugs.gnu.org,
> > 	salutis <at> me.com
> > 
> > In this case the call to ENCODE_FILE in allocInitFromFile is actually
> > redundant because image_find_image_fd already calls ENCODE_FILE on the
> > filename before passing it back. So we get a UTF-8 string no matter
> > what.
> 
> Then why was the code re-encoding t in UTF-16?  A bug?

No, sorry, I'm not being clear. The internal format of NSString is
UTF-16. We can load practically anything into it, as long as we know
ahead of time what the encoding is.

> > NSString can read in almost anything, and Mattias extended it to read
> > in multibyte (and ascii) lisp strings, so we don't need a UTF-16 input
> > specifically. It would probably be nice if NSString was also able to
> > recognise that a lisp string is UTF-8 and handle that itself, but I
> > don't think that's really possible, unless we make the assumption that
> > any unibyte string it's passed will already be ascii or UTF-8.
> > 
> > I don't know if that's a reasonable assumption.
> 
> Any file name passed through ENCODE_FILE should be in UTF-8 on macOS,
> as I understand that's how the macOS filesystems work.  Am I mistaken?
> Can the value of default-file-name-coding-system on macOS be anything
> other than UTF-8?

Not as far as I'm aware. But NSString is used all over the place in
the NS port code base (and all through the toolkit). Any time we pass
a string to or from the toolkit it has to be converted to or from
NSString. I think most of the actual file access code in Emacs is low
level C code which won't go near NSString, though, so it's not worth
changing C code to make ObjC code cleaner.

I guess I'm just being lazy and would like our extensions to NSString
to just DTRT, but it seems that's impractical. :)
-- 
Alan Third

Message #68 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Alan Third <alan <at> idiocy.org>
Cc: 48902 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Rudolf Adamkovič <salutis <at> me.com>, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 8 Jun 2021 21:52:51 +0200

8 juni 2021 kl. 21.10 skrev Alan Third <alan <at> idiocy.org>:

> In my defence it wasn't entirely clear to me that a lisp string
> returned from ENCODE_FILE was incompatible with stringWithLispString. ;)

Oh it's compatible all right, it just takes it job description very literally!
That's typical of them computers -- no imagination at all.

> Hmm, and as you point out we use "file" further down and it may or may
> not be encoded, but will probably have the same contents as found,
> which we know is encoded. Plus it's setting the "name" field in the
> image, which we probably want to keep as uniform as possible for
> caching purposes but is otherwise irrelevant.
> 
> 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.

Message #71 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: "naofumi <at> yasufuku.dev" <naofumi <at> yasufuku.dev>
Cc: 48902 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, salutis <at> me.com,
 larsi <at> gnus.org, alan <at> idiocy.org
Subject: Re: bug#48902: 28.0.50; Directory  names containing apostrophes and
 backticks cause problems
Date: Tue, 8 Jun 2021 22:08:19 +0200

8 juni 2021 kl. 21.13 skrev naofumi <at> yasufuku.dev:

> For example, another patch using STRING_SET_MULTIBYTE() is here.
> This looks still just a quick fix and bit weired, though..

Thank you, but it's probably better to always return an unencoded string from image_find_image_fd in that case. The current code looks like a premature optimisation.

> On the other hand, I cannot find out that non-UTF-8 filename coding is
> really needed on macOS.  It might be over-engineered thing, and just an overhead.

Maybe and in practice you are probably right, but the NS port is not exclusive to macOS.
There is the quasi-NFD normalisation step but I'm not sure how important that is today. There's no need to convert to NFD for accessing files; it only matters when comparing names (much like case folding on many file systems).

Message #74 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 48902 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Rudolf Adamkovič <salutis <at> me.com>, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Tue, 8 Jun 2021 21:33:01 +0100

[Message part 1 (text/plain, inline)]

On Tue, Jun 08, 2021 at 09:52:51PM +0200, Mattias Engdegård wrote:
> 8 juni 2021 kl. 21.10 skrev Alan Third <alan <at> 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

[v2-0001-Fix-image-filename-encoding-issues-bug-48902.patch (text/x-diff, attachment)]

Message #77 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Rudolf Adamkovič <salutis <at> me.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Wed, 09 Jun 2021 00:21:42 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

> When you visit that directory in Dired, what is the value of
> buffer-file-coding-system in the Dired buffer?

The value is “utf-8-unix”.

> Also, you mention backtick, but there's no such character in the
> example you show.

There is no backtick. My apologies!

-- Rudy

Message #80 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Alan Third <alan <at> idiocy.org>
Cc: 48902 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Rudolf Adamkovič <salutis <at> me.com>, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Wed, 9 Jun 2021 13:40:18 +0200

8 juni 2021 kl. 22.33 skrev Alan Third <alan <at> idiocy.org>:

> 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.

Thank you, and I arrived at the same conclusion.

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

Looks fine, but the image_find_image_file comment needs to be amended since it says that it returns an encoded string.

> 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?

I should think so -- stringWithLispString: was designed as a general-purpose method to convert from a lisp string to NSString without changing the contents. Non-Unicode values (which includes raw bytes) become U+FFFD except surrogates as they can be represented (in a manner of speaking) in UTF-16, and it turns out to be more useful that way.

Furthermore, if we use stringWithLispString: for file names, no special file name encoding step should be needed on our side, since the NS libs will perform any needed normalisation (at least if I've understood it right).

Message #83 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Third <alan <at> idiocy.org>
Cc: 48902 <at> debbugs.gnu.org, mattiase <at> acm.org, larsi <at> gnus.org, salutis <at> me.com,
 naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50;
 Directory names containing apostrophes and backticks cause problems
Date: Wed, 09 Jun 2021 14:56:14 +0300

> Date: Tue, 8 Jun 2021 21:33:01 +0100
> From: Alan Third <alan <at> idiocy.org>
> Cc: 48902 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
>  Rudolf Adamkovič <salutis <at> me.com>, naofumi <at> yasufuku.dev
> 
> 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.

Encoding an already encoded file name is a no-op.  But I agree it's
unclean and we had better fixed that.

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

Thanks, it LGTM.  But please also adjust the comment of
image_find_image_fd.

Message #86 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 48902 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Rudolf Adamkovič <salutis <at> me.com>, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Wed, 9 Jun 2021 16:19:35 +0100

On Wed, Jun 09, 2021 at 01:40:18PM +0200, Mattias Engdegård wrote:
> 8 juni 2021 kl. 22.33 skrev Alan Third <alan <at> idiocy.org>:
> 
> > Patch attached. I've tested it here but I only have a couple of images
> > to try it with.
> 
> Looks fine, but the image_find_image_file comment needs to be
> amended since it says that it returns an encoded string.

I've made a change to the comment.

> Furthermore, if we use stringWithLispString: for file names, no
> special file name encoding step should be needed on our side, since
> the NS libs will perform any needed normalisation (at least if I've
> understood it right).

Yes, I believe that's right, so I've made that change too and pushed
to master.

As far as I can see this fixes the problem so I'll close the bug
report. If it's still broken in some way, please reply to this email
and we'll reopen the bug.

Thanks everyone!
-- 
Alan Third

Message #91 received at 48902 <at> debbugs.gnu.org (full text, mbox):

From: Rudolf Adamkovič <salutis <at> me.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 48902 <at> debbugs.gnu.org,
 Mattias Engdegård <mattiase <at> acm.org>,
 Lars Ingebrigtsen <larsi <at> gnus.org>, naofumi <at> yasufuku.dev
Subject: Re: bug#48902: 28.0.50; Directory names containing apostrophes and
 backticks cause problems
Date: Sat, 12 Jun 2021 00:09:17 +0200

I have pulled, recompiled, updated my EMMS music library, and now, 
every album has artwork showing, including Japanese and other 
non-English albums. Thank you Alan, Eli, Lars, Mattias, and 
Naofumi. This was my first GNU bug report, and, as a fresh 
GNU/Emacs/DRM-free convert, I found the experience rather 
impressive!

R+

Alan Third <alan <at> idiocy.org> writes:

> On Wed, Jun 09, 2021 at 01:40:18PM +0200, Mattias Engdegård 
> wrote:
>> 8 juni 2021 kl. 22.33 skrev Alan Third <alan <at> idiocy.org>:
>>
>> > Patch attached. I've tested it here but I only have a couple 
>> > of images
>> > to try it with.
>>
>> Looks fine, but the image_find_image_file comment needs to be
>> amended since it says that it returns an encoded string.
>
> I've made a change to the comment.
>
>> Furthermore, if we use stringWithLispString: for file names, no
>> special file name encoding step should be needed on our side, 
>> since
>> the NS libs will perform any needed normalisation (at least if 
>> I've
>> understood it right).
>
> Yes, I believe that's right, so I've made that change too and 
> pushed
> to master.
>
> As far as I can see this fixes the problem so I'll close the bug
> report. If it's still broken in some way, please reply to this 
> email
> and we'll reopen the bug.
>
> Thanks everyone!

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.