bug#50946: insert-file-contents can corrupt buffers. [Was: bug#50946: Emacs-28: Inadequate coding in hack-elisp-shorthands]

[Alan Mackenzie]

On Sat, Oct 02, 2021 at 18:00:38 +0300, Eli Zaretskii wrote:
> > Date: Sat, 2 Oct 2021 14:45:52 +0000
> > Cc: joaotavora@gmail.com, 50946@debbugs.gnu.org
> > From: Alan Mackenzie <acm@muc.de>

[ .... ]

> > Have you checked that things work if the first byte in your temporary
> > buffer isn't at the start of a character?

> I don't see why this matter, can you explain?

Yes, it does matter.  Because....

Create a file utf8-chars.txt as follows.  All the non-ascii characters
are 2-byte German UTF8 characters:
Yes, it does matter.  Because....

Create a file ~/utf8-chars.txt as follows.  All the non-ascii characters
are 2-byte German UTF8 characters.  Only the Q is an ascii character.
There is a LF at the end:

ÄäÖöQÜüß

Now, in an empty buffer,

   M-: (insert-file-contents "~/utf8-chars.txt" nil 3 15)

..  The first character of this buffer is now the Emacs encoding of the
raw byte 0xa4.

Now do

   M-: (insert-file-contents "~/utf8-chars.txt" nil 0 3)

The entire buffer, apart from the Q and the LF, now consists of raw
bytes, and the buffer is now 16 characters long.  (Is this a bug?).
Note that the Q is now further back from the end of the buffer than it
should be.

Using insert-file-contents-literally instead doesn't help.

So insert-file-contents corrupts the buffer when BEG or END is not at a
character boundary.  This matters for hack-elisp-shorthands, because this
corruption could push the Local Variables: start further back than 3000
characters.  Possibly other problems could happen, too.

My opinion, for what it's worth, is that using insert-file-contents in
hack-elisp-shorthands is a Bad Thing.  Even if it is possible to get it
working rigorously, it is surely not worth the trouble.  Why not simply
visit the file in a buffer, and check for buffer local variables in the
normal fashion?

#########################################################################

There are bugs in the documentation of insert-file-contents in the elisp
manual.  It confuses bytes with characters, and it fails to mention the
need to keep BEG and END at character boundaries.  I propose installing
the following patch to the release branch:



diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 2dc808e694..c344e18c2b 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -556,6 +556,9 @@ Reading from Files
 
 If @var{beg} and @var{end} are non-@code{nil}, they should be numbers
 that are byte offsets specifying the portion of the file to insert.
+Be careful to ensure that these byte positions are at character
+boundaries.  Otherwise, Emacs's input functions will corrupt the
+buffer.
 In this case, @var{visit} must be @code{nil}.  For example,
 
 @example
@@ -563,7 +566,7 @@ Reading from Files
 @end example
 
 @noindent
-inserts the first 500 characters of a file.
+inserts the characters coded by the first 500 bytes of a file.
 
 If the argument @var{replace} is non-@code{nil}, it means to replace the
 contents of the buffer (actually, just the accessible portion) with the
@@ -580,7 +583,8 @@ Reading from Files
 This function works like @code{insert-file-contents} except that it
 does not run @code{after-insert-file-functions}, and does not do
 format decoding, character code conversion, automatic uncompression,
-and so on.
+and so on.  @var{beg} and @var{end}, if non-@code{nil}, should be at
+character boundaries, as in @code{insert-file-contents}.
 @end defun
 
 If you want to pass a file name to another process so that another


The doc strings of insert-file-contents\(-literally\)? will also need to
be updated.

-- 
Alan Mackenzie (Nuremberg, Germany).