bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Robert Pluim]

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

mm-inline-media-tests contains lambda's, and itʼs byte-compiled, so
when you customize its value, byte-code is saved in your .emacs. One
of its components looks like this:

     ("text/x-vcard" mm-inline-text-vcard
      #[257 "\300\301!\206	%0\302\303!\207"
            [featurep vcard locate-library "vcard"]
            3 "

     (fn HANDLE)"])

That has an embedded 'NUL' just before the \302 (Iʼve encoded it),
which causes 'buffer-file-coding-system' to be incorrect
('no-conversion in this case) so any non-ascii character causes a read
error:

$ HOME=/tmp emacs --debug-init

Debugger entered--Lisp error: (invalid-read-syntax "?" 1 0)
  read(#<buffer  *load*>)
  eval-buffer(#<buffer  *load*> nil "/tmp/.emacs" nil t)  ; Reading at buffer position 573
  load-with-code-conversion("/tmp/.emacs" "/tmp/.emacs" t t)
  load("~/.emacs" noerror nomessage)

Sample .emacs attached. There are probably other defcustom's with the
same issue, I haven't checked.


[-- Attachment #2: .emacs --]
[-- Type: application/octet-stream, Size: 577 bytes --]

[-- Attachment #3: Type: text/plain, Size: 279 bytes --]


In GNU Emacs 28.0.90 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-12-13 built on rltb
Repository revision: 9bd3f78645e14fdbaf3a569df5e0a52249c4f90e
Repository branch: emacs-28
System Description: Debian GNU/Linux 11 (bullseye)

Robert
-- 

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Thu, 16 Dec 2021 16:18:13 +0100
> 
> That has an embedded 'NUL' just before the \302 (Iʼve encoded it),
> which causes 'buffer-file-coding-system' to be incorrect
> ('no-conversion in this case) so any non-ascii character causes a read
> error:
> 
> $ HOME=/tmp emacs --debug-init
> 
> Debugger entered--Lisp error: (invalid-read-syntax "?" 1 0)
>   read(#<buffer  *load*>)
>   eval-buffer(#<buffer  *load*> nil "/tmp/.emacs" nil t)  ; Reading at buffer position 573
>   load-with-code-conversion("/tmp/.emacs" "/tmp/.emacs" t t)
>   load("~/.emacs" noerror nomessage)

Perhaps we should bind inhibit-null-byte-detection to a non-nil value
while loading the init file?

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Robert Pluim]

>>>>> On Thu, 16 Dec 2021 17:30:27 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Date: Thu, 16 Dec 2021 16:18:13 +0100
    >> 
    >> That has an embedded 'NUL' just before the \302 (Iʼve encoded it),
    >> which causes 'buffer-file-coding-system' to be incorrect
    >> ('no-conversion in this case) so any non-ascii character causes a read
    >> error:
    >> 
    >> $ HOME=/tmp emacs --debug-init
    >> 
    >> Debugger entered--Lisp error: (invalid-read-syntax "?" 1 0)
    >> read(#<buffer  *load*>)
    >> eval-buffer(#<buffer  *load*> nil "/tmp/.emacs" nil t)  ; Reading at buffer position 573
    >> load-with-code-conversion("/tmp/.emacs" "/tmp/.emacs" t t)
    >> load("~/.emacs" noerror nomessage)

    Eli> Perhaps we should bind inhibit-null-byte-detection to a non-nil value
    Eli> while loading the init file?

The init file and 'custom-file'? (anyone who manually puts byte-code
in their .emacs either knows what theyʼre doing or will soon learn :-) )

Robert
-- 

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Andreas Schwab]

On Dez 16 2021, Eli Zaretskii wrote:

>> From: Robert Pluim <rpluim@gmail.com>
>> Date: Thu, 16 Dec 2021 16:18:13 +0100
>> 
>> That has an embedded 'NUL' just before the \302 (Iʼve encoded it),
>> which causes 'buffer-file-coding-system' to be incorrect
>> ('no-conversion in this case) so any non-ascii character causes a read
>> error:
>> 
>> $ HOME=/tmp emacs --debug-init
>> 
>> Debugger entered--Lisp error: (invalid-read-syntax "?" 1 0)
>>   read(#<buffer  *load*>)
>>   eval-buffer(#<buffer  *load*> nil "/tmp/.emacs" nil t)  ; Reading at buffer position 573
>>   load-with-code-conversion("/tmp/.emacs" "/tmp/.emacs" t t)
>>   load("~/.emacs" noerror nomessage)
>
> Perhaps we should bind inhibit-null-byte-detection to a non-nil value
> while loading the init file?

I'd rather think that custom-save-variable should bind
print-escape-control-characters to t.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: 52554@debbugs.gnu.org
> Date: Thu, 16 Dec 2021 16:46:36 +0100
> 
>     Eli> Perhaps we should bind inhibit-null-byte-detection to a non-nil value
>     Eli> while loading the init file?
> 
> The init file and 'custom-file'?

Yep.

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Robert Pluim <rpluim@gmail.com>,  52554@debbugs.gnu.org
> Date: Thu, 16 Dec 2021 17:13:53 +0100
> 
> On Dez 16 2021, Eli Zaretskii wrote:
> 
> >> $ HOME=/tmp emacs --debug-init
> >> 
> >> Debugger entered--Lisp error: (invalid-read-syntax "?" 1 0)
> >>   read(#<buffer  *load*>)
> >>   eval-buffer(#<buffer  *load*> nil "/tmp/.emacs" nil t)  ; Reading at buffer position 573
> >>   load-with-code-conversion("/tmp/.emacs" "/tmp/.emacs" t t)
> >>   load("~/.emacs" noerror nomessage)
> >
> > Perhaps we should bind inhibit-null-byte-detection to a non-nil value
> > while loading the init file?
> 
> I'd rather think that custom-save-variable should bind
> print-escape-control-characters to t.

Maybe we should do both?

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Robert Pluim]

>>>>> On Thu, 16 Dec 2021 18:58:19 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Andreas Schwab <schwab@linux-m68k.org>
    >> Cc: Robert Pluim <rpluim@gmail.com>,  52554@debbugs.gnu.org
    >> Date: Thu, 16 Dec 2021 17:13:53 +0100
    >> 
    >> On Dez 16 2021, Eli Zaretskii wrote:
    >> 
    >> >> $ HOME=/tmp emacs --debug-init
    >> >> 
    >> >> Debugger entered--Lisp error: (invalid-read-syntax "?" 1 0)
    >> >>   read(#<buffer  *load*>)
    >> >>   eval-buffer(#<buffer  *load*> nil "/tmp/.emacs" nil t)  ; Reading at buffer position 573
    >> >>   load-with-code-conversion("/tmp/.emacs" "/tmp/.emacs" t t)
    >> >>   load("~/.emacs" noerror nomessage)
    >> >
    >> > Perhaps we should bind inhibit-null-byte-detection to a non-nil value
    >> > while loading the init file?
    >> 
    >> I'd rather think that custom-save-variable should bind
    >> print-escape-control-characters to t.

    Eli> Maybe we should do both?

In `custom-save-all' perhaps? There are many code paths for saving
customizations, they donʼt all go via `custom-save-variable'.

Robert
-- 

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Andreas Schwab <schwab@linux-m68k.org>,  52554@debbugs.gnu.org
> Date: Fri, 17 Dec 2021 09:23:48 +0100
> 
>     >> I'd rather think that custom-save-variable should bind
>     >> print-escape-control-characters to t.
> 
>     Eli> Maybe we should do both?
> 
> In `custom-save-all' perhaps? There are many code paths for saving
> customizations, they donʼt all go via `custom-save-variable'.

If they all go through custom-save-all, then I think it should be done
there, yes.

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Robert Pluim]

>>>>> On Fri, 17 Dec 2021 10:29:09 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: Andreas Schwab <schwab@linux-m68k.org>,  52554@debbugs.gnu.org
    >> Date: Fri, 17 Dec 2021 09:23:48 +0100
    >> 
    >> >> I'd rather think that custom-save-variable should bind
    >> >> print-escape-control-characters to t.
    >> 
    Eli> Maybe we should do both?
    >> 
    >> In `custom-save-all' perhaps? There are many code paths for saving
    >> customizations, they donʼt all go via `custom-save-variable'.

    Eli> If they all go through custom-save-all, then I think it should be done
    Eli> there, yes.

OK, Iʼll run with the following for a while, and see if anything crops
up.

diff --git c/lisp/cus-edit.el i/lisp/cus-edit.el
index 5c4448ae71..1936733098 100644
--- c/lisp/cus-edit.el
+++ i/lisp/cus-edit.el
@@ -4723,7 +4723,10 @@ custom-save-all
         (delay-mode-hooks (emacs-lisp-mode)))
       (let ((inhibit-read-only t)
 	    (print-length nil)
-	    (print-level nil))
+	    (print-level nil)
+            ;; We might be saving byte-code with embedded NULs, which can
+            ;; cause problems when read back (Bug#52554).
+            (print-escape-control-characters t))
         (atomic-change-group
 	  (custom-save-variables)
 	  (custom-save-faces)))
diff --git c/lisp/startup.el i/lisp/startup.el
index 505d7b83f4..f8db46ff3f 100644
--- c/lisp/startup.el
+++ i/lisp/startup.el
@@ -964,7 +964,11 @@ startup--load-user-init-file
         (debug-on-error-initial
          (if (eq init-file-debug t)
              'startup
-           init-file-debug)))
+           init-file-debug))
+        ;; The init file might contain byte-code with embedded NULs,
+        ;; which can cause problems when read back, so disable null
+        ;; byte detection (Bug#52554).
+        (inhibit-null-byte-detection t))
     (let ((debug-on-error debug-on-error-initial))
       (condition-case-unless-debug error
           (when init-file-user

Robert
-- 

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: schwab@linux-m68k.org,  52554@debbugs.gnu.org
> Date: Fri, 17 Dec 2021 09:45:17 +0100
> 
>     >> >> I'd rather think that custom-save-variable should bind
>     >> >> print-escape-control-characters to t.
>     >> 
>     Eli> Maybe we should do both?
>     >> 
>     >> In `custom-save-all' perhaps? There are many code paths for saving
>     >> customizations, they donʼt all go via `custom-save-variable'.
> 
>     Eli> If they all go through custom-save-all, then I think it should be done
>     Eli> there, yes.
> 
> OK, Iʼll run with the following for a while, and see if anything crops
> up.

Thanks.

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> mm-inline-media-tests contains lambda's, and itʼs byte-compiled, so
> when you customize its value, byte-code is saved in your .emacs. One
> of its components looks like this:
>
>      ("text/x-vcard" mm-inline-text-vcard
>       #[257 "\300\301!\206	%0\302\303!\207"
>             [featurep vcard locate-library "vcard"]
>             3 "
>
>      (fn HANDLE)"])

I see that you've fixed the actual bug in a later patch, but I just
wanted to ask -- should that variable be a user option at all?  Long
list variables are seldom useful as user options.  Instead users should
prepend choices with `push' or something.

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

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> It is kind of awkward to use, but then again, I only wanted to delete
> an entry, and itʼs not something Iʼd do often. The embedded code
> bothers me more: customize is supposed to be about configuration
> variables.

Yup.  I'm not sure how to express stuff like

    ("image/png"
     mm-inline-image
     ,(lambda (handle)
       (mm-valid-and-fit-image-p 'png handle)))

and

    ("text/x-vcard"
     mm-inline-text-vcard
     ,(lambda (_handle)
       (or (featurep 'vcard)
	   (locate-library "vcard"))))

in a defcustom-friendly way without having an explosion of helper
functions, though.

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

bug#52554: 28.0.90; mm-inline-media-tests customization breaks .emacs reading

[Robert Pluim]

tags 52554 fixed
close 52554 29.1
quit

>>>>> On Fri, 17 Dec 2021 14:11:06 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> 
    >> OK, Iʼll run with the following for a while, and see if anything crops
    >> up.

    Eli> Thanks.

I haven't seen any negative effect so far, so:

Closing.
Committed as a86205b060