bug#43723: 27.1; Errors in file-extended-attributes prevent from saving buffer

bug#43723: 27.1; Errors in file-extended-attributes prevent from saving buffer

[Eli Zaretskii]

When some volume is mounted such that file-extended-attributes fails
for files there (because the agent used to mount doesn't support ACLs
or SELinux), this prevents users from saving their edits to files on
that volume, because file-extended-attributes signals an error.  This
appears as a regression to users because Emacs 26 silently ignored
such errors.

To allow users to save the files in these cases, but still keep them
informed about the loss of potentially important attributes, Emacs
should probably warn about this, allow the user to decide he/she wants
to ignore the problem, and record this fact somewhere, to avoid asking
the same question again for the same volume.

Bonus points for providing a defcustom where users could state in
advance the volumes for which such errors should be ignored.


In GNU Emacs 27.1 (build 1, i686-pc-mingw32)
 of 2020-08-11 built on HOME-C4E4A596F7
Windowing system distributor 'Microsoft Corp.', version 5.1.2600
System Description: Microsoft Windows XP Service Pack 3 (v5.1.0.2600)

Recent messages:
Saving file d:/gnu/git/emacs/trunk/.git/COMMIT_EDITMSG...
Wrote d:/gnu/git/emacs/trunk/.git/COMMIT_EDITMSG
Mark set [7 times]
Sending...
Added to d:/usr/eli/rmail/SENT.MAIL
Sending email done
Sending...done
Showing message 2726...done
Showing message 2726...done
Modification-flag cleared

Configured using:
 'configure -C --prefix=/d/usr --with-wide-int --with-modules
 'CFLAGS=-O2 -gdwarf-4 -g3''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY W32NOTIFY ACL GNUTLS LIBXML2
HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS MODULES THREADS JSON PDUMPER LCMS2
GMP

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1255

Major mode: RMAIL

Minor modes in effect:
  shell-dirtrack-mode: t
  desktop-save-mode: t
  save-place-mode: t
  show-paren-mode: t
  display-battery-mode: t
  display-time-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  buffer-read-only: t
  line-number-mode: t

Load-path shadows:
d:/usr/share/emacs/site-lisp/soap-inspect hides d:/usr/share/emacs/27.1/lisp/net/soap-inspect
d:/usr/share/emacs/site-lisp/soap-client hides d:/usr/share/emacs/27.1/lisp/net/soap-client

Features:
(shadow emacsbug apropos tabify grep compile ibuf-ext ibuffer
ibuffer-loaddefs arc-mode archive-mode debug backtrace hi-lock double
mouse-copy mouse-drag m4-mode xscheme scheme tramp tramp-loaddefs
trampver tramp-integration files-x tramp-compat parse-time iso8601
pulse etags fileloop generator skeleton face-remap image-mode exif
completion cua-gmrk cua-rect rect cua-base pp descr-text ffap
ebuff-menu iso-transl smerge-mode diff quail cl-print misearch
multi-isearch eieio-opt speedbar sb-image ezimage dframe find-func
thingatpt help-fns radix-tree dabbrev rmailout shr-color color shr
url-cookie url-domsuf url-util url-parse url-vars svg xml dom
browse-url shell pcomplete comint ansi-color rfc2104 gnutls
network-stream nsm mail-extr smtpmail auth-source json map mailalias
sendmail cl-extra help-mode cc-awk view enriched conf-mode texinfo
make-mode jka-compr vc-cvs bat-mode vc-dispatcher vc-bzr noutline
outline flyspell add-log vc-git diff-mode easy-mmode bug-reference
mule-util info qp rmailsum rmailmm message rmc puny format-spec rfc822
mml mml-sec password-cache epa epg epg-config gnus-util
text-property-search time-date subr-x seq mm-decode mm-bodies
mm-encode mailabbrev gmm-utils mailheader mail-parse rfc2231 rmail
rmail-loaddefs rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils desktop frameset server find-lisp dired dired-loaddefs
filecache mairix cus-edit cus-start cus-load wid-edit saveplace
midnight ispell derived generic-x cc-mode cc-fonts easymenu cc-guess
cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs paren
xref cl-seq project ring eieio byte-opt bytecomp byte-compile cconv
eieio-core cl-macs gv eieio-loaddefs cl-loaddefs cl-lib battery time
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel dos-w32 ls-lisp disp-table term/w32-win w32-win w32-vars
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 timer
select scroll-bar mouse jit-lock font-lock syntax facemenu 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
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads w32notify w32 lcms2
multi-tty make-network-process emacs)

Memory information:
((conses 16 4156244 469197)
 (symbols 48 40145 57)
 (strings 16 316024 26318)
 (string-bytes 1 7689602)
 (vectors 16 73714)
 (vector-slots 8 2294162 512108)
 (floats 8 324 643)
 (intervals 40 922715 7602)
 (buffers 888 319))

bug#43723: 27.1; Errors in file-extended-attributes prevent from saving buffer

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> When some volume is mounted such that file-extended-attributes fails
> for files there (because the agent used to mount doesn't support ACLs
> or SELinux), this prevents users from saving their edits to files on
> that volume, because file-extended-attributes signals an error.  This
> appears as a regression to users because Emacs 26 silently ignored
> such errors.
>
> To allow users to save the files in these cases, but still keep them
> informed about the loss of potentially important attributes, Emacs
> should probably warn about this, allow the user to decide he/she wants
> to ignore the problem, and record this fact somewhere, to avoid asking
> the same question again for the same volume.

Do you have a test case to reproduce this problem?  I don't really use
SELinux myself...

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

bug#43723: 27.1; Errors in file-extended-attributes prevent from saving buffer

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 43723@debbugs.gnu.org
> Date: Mon, 06 Jun 2022 18:22:13 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > When some volume is mounted such that file-extended-attributes fails
> > for files there (because the agent used to mount doesn't support ACLs
> > or SELinux), this prevents users from saving their edits to files on
> > that volume, because file-extended-attributes signals an error.  This
> > appears as a regression to users because Emacs 26 silently ignored
> > such errors.
> >
> > To allow users to save the files in these cases, but still keep them
> > informed about the loss of potentially important attributes, Emacs
> > should probably warn about this, allow the user to decide he/she wants
> > to ignore the problem, and record this fact somewhere, to avoid asking
> > the same question again for the same volume.
> 
> Do you have a test case to reproduce this problem?  I don't really use
> SELinux myself...

No, I don't have a recipe.

This bug report was the result of this discussion on emacs-devel:

  https://lists.gnu.org/archive/html/emacs-devel/2020-09/msg02248.html

The basic concern is that Emacs 26 silently ignored errors in file-acl
and file-selinux-context, whereas Emacs 27 and later doesn't ignore
them.  My point was that preventing the user from saving the edits
just because we cannot preserve the ACLs is too radical, since most
users don't care about ACLs, and because support for ACLs on volumes
mounted by all kinds of network disk drivers that have trouble mapping
extended attributes between different systems.

You can easily simulate this situation by writing a replacement for
file-acl that always signals a file-error, or advising it to that
effect.

bug#43723: 27.1; Errors in file-extended-attributes prevent from saving buffer

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> The basic concern is that Emacs 26 silently ignored errors in file-acl
> and file-selinux-context, whereas Emacs 27 and later doesn't ignore
> them.  My point was that preventing the user from saving the edits
> just because we cannot preserve the ACLs is too radical, since most
> users don't care about ACLs, and because support for ACLs on volumes
> mounted by all kinds of network disk drivers that have trouble mapping
> extended attributes between different systems.

I see.

> You can easily simulate this situation by writing a replacement for
> file-acl that always signals a file-error, or advising it to that
> effect.

It's always better to have an actual test case so that we're not fixing
the wrong thing...

But in this case, I guess it's `set-file-extended-attributes' that
should be signalling an error if we want to emulate the problem?
`file-acl' would presumably just return nil in these cases, so there'd
be no warnings or errors?

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

bug#43723: 27.1; Errors in file-extended-attributes prevent from saving buffer

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 43723@debbugs.gnu.org
> Date: Tue, 07 Jun 2022 11:26:17 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > You can easily simulate this situation by writing a replacement for
> > file-acl that always signals a file-error, or advising it to that
> > effect.
> 
> It's always better to have an actual test case so that we're not fixing
> the wrong thing...

Yes, but all of my actual examples were fixed "by other means".  So
only the theoretical issue is left.

> But in this case, I guess it's `set-file-extended-attributes' that
> should be signalling an error if we want to emulate the problem?
> `file-acl' would presumably just return nil in these cases, so there'd
> be no warnings or errors?

No, the problem is exactly that file-extended-attributes signals an
error.  The set-file-extended-attributes part is handled correctly:

	       ;; If set-file-extended-attributes fails, fall back on
	       ;; set-file-modes.
	       (unless
		   (with-demoted-errors "Error setting attributes: %s"
		     (set-file-extended-attributes buffer-file-name
						   (nth 1 setmodes)))
		 (set-file-modes buffer-file-name
				 (logior (car setmodes) 128)))))

Which to me is just one more argument in favor of showing a simple
message when file-extended-attributes fails, instead of signaling an
error and breaking the save-buffer command.

bug#43723: 27.1; Errors in file-extended-attributes prevent from saving buffer

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> No, the problem is exactly that file-extended-attributes signals an
> error.  The set-file-extended-attributes part is handled correctly:
>
> 	       ;; If set-file-extended-attributes fails, fall back on
> 	       ;; set-file-modes.
> 	       (unless
> 		   (with-demoted-errors "Error setting attributes: %s"
> 		     (set-file-extended-attributes buffer-file-name
> 						   (nth 1 setmodes)))
> 		 (set-file-modes buffer-file-name
> 				 (logior (car setmodes) 128)))))

That code is for when the file isn't writable, so it's not normally
encountered.  (And apparently doesn't do anything anyway, see the other
bug report about this...)

> Which to me is just one more argument in favor of showing a simple
> message when file-extended-attributes fails, instead of signaling an
> error and breaking the save-buffer command.

So it's this bit that bugs out in obscure situations?

	    ;; Since we have created an entirely new file,
	    ;; make sure it gets the right permission bits set.
	    (setq setmodes (or setmodes
 			       (list (or (file-modes buffer-file-name)
					 (logand ?\666 (default-file-modes)))
				     (file-extended-attributes buffer-file-name)
				     buffer-file-name)))

Since this is such an unlikely thing to error out, perhaps just doing a
`with-demoted-errors' here is sufficient?

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

bug#43723: 27.1; Errors in file-extended-attributes prevent from saving buffer

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 43723@debbugs.gnu.org
> Date: Tue, 07 Jun 2022 13:52:27 +0200
> 
> So it's this bit that bugs out in obscure situations?
> 
> 	    ;; Since we have created an entirely new file,
> 	    ;; make sure it gets the right permission bits set.
> 	    (setq setmodes (or setmodes
>  			       (list (or (file-modes buffer-file-name)
> 					 (logand ?\666 (default-file-modes)))
> 				     (file-extended-attributes buffer-file-name)
> 				     buffer-file-name)))
> 
> Since this is such an unlikely thing to error out, perhaps just doing a
> `with-demoted-errors' here is sufficient?

Probably.

bug#43723: 27.1; Errors in file-extended-attributes prevent from saving buffer

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> Since this is such an unlikely thing to error out, perhaps just doing a
>> `with-demoted-errors' here is sufficient?
>
> Probably.

I instrumented file-extended-attributes to fail and then demoted the two
reads in basic-save-buffer-2 in Emacs 29.

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