bug#64686: 30.0.50; Tar mode @PaxHeader bug

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Devon Sean McCullough]

$ wget 
https://f-droid.org/repo/com.simplemobiletools.voicerecorder_36_src.tar.gz
$ emacs --no-init-file --no-site-file 
com.simplemobiletools.voicerecorder_36_src.tar.gz

should show something like the output of

$ tar -tvz < com.simplemobiletools.voicerecorder_36_src.tar.gz

instead of spurious ././@PaxHeader lines
interleaved with truncated long file names.

         Peace
             --Devon

P.S.  In case it matters:
$ tar --version
tar (GNU tar) 1.34
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
<https://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by John Gilmore and Jay Fenlason.
$ type tar
tar is hashed (/usr/bin/tar)
$ ldd /usr/bin/tar
     linux-vdso.so.1 (0x00007ffca7b51000)
     libacl.so.1 => /lib/x86_64-linux-gnu/libacl.so.1 (0x00007fcd50638000)
     libselinux.so.1 => /lib/x86_64-linux-gnu/libselinux.so.1 
(0x00007fcd5060c000)
     libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fcd50438000)
     libpcre2-8.so.0 => /lib/x86_64-linux-gnu/libpcre2-8.so.0 
(0x00007fcd503a0000)
     libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fcd5039a000)
     /lib64/ld-linux-x86-64.so.2 (0x00007fcd506e2000)
     libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 
(0x00007fcd50378000)

In GNU Emacs 30.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version
  3.24.27, cairo version 1.16.0) of 2023-05-10 built on LQ
Repository revision: e4c8ba6c0589f1ba0ba9993c4f1e9c0821d3ca6c
Repository branch: master
System Description: PureOS 10 (Byzantium)

Configured using:
  'configure --with-pgtk'

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ LIBSELINUX
MODULES NOTIFY INOTIFY PDUMPER PGTK PNG SECCOMP SOUND THREADS
TOOLKIT_SCROLL_BARS XIM GTK3 ZLIB

Important settings:
   value of $LC_MONETARY: fr_FR.UTF-8
   value of $LC_NUMERIC: fr_FR.UTF-8
   value of $LC_TIME: fr_FR.UTF-8
   value of $LANG: en_US.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Tar

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   show-paren-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
   buffer-read-only: t
   line-number-mode: t
   indent-tabs-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils tar-mode arc-mode archive-mode jka-compr rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/pgtk-win pgtk-win
term/common-win pgtk-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-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 nadvice seq simple cl-generic indonesian philippine
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 emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo gtk pgtk multi-tty make-network-process emacs)

Memory information:
((conses 16 46611 11513)
  (symbols 48 5497 0)
  (strings 32 16893 1459)
  (string-bytes 1 478022)
  (vectors 16 12517)
  (vector-slots 8 186004 14041)
  (floats 8 26 296)
  (intervals 56 2802 5)
  (buffers 976 13))

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Eli Zaretskii]

> Date: Mon, 17 Jul 2023 14:31:39 -0400
> From: Devon Sean McCullough <emacs-hacker2017@jovi.net>
> 
> $ wget 
> https://f-droid.org/repo/com.simplemobiletools.voicerecorder_36_src.tar.gz
> $ emacs --no-init-file --no-site-file 
> com.simplemobiletools.voicerecorder_36_src.tar.gz
> 
> should show something like the output of
> 
> $ tar -tvz < com.simplemobiletools.voicerecorder_36_src.tar.gz
> 
> instead of spurious ././@PaxHeader lines
> interleaved with truncated long file names.

Thanks.  In a nutshell, tar-mode didn't support Posix-standard pax
extended headers.  Should be fixed now on the master branch.

P.S. Thanks to Stefan who found a subtle bug in the prototype of this
fix.

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Mattias Engdegård]

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

The change (2dc5f17c3e) causes packages-tests to fail; see attached log.
This is on macOS with an out-of-tree build.


[-- Attachment #2: package-tests.log --]
[-- Type: application/octet-stream, Size: 3453 bytes --]

  GEN      lisp/emacs-lisp/package-tests.log
Running 37 tests (2023-07-27 16:57:37+0200, selector `(not (or (tag :unstable) (tag :nativecomp)))')
Parsing tar file... 
Parsing tar file...done
Parsing tar file... 
Parsing tar file...done
Parsing tar file... 
Parsing tar file...done
Extracting... \ 
Extracting...done
  INFO     Scraping files for loaddefs... 
  INFO     Scraping files for loaddefs...done
  GEN      ustar-withsub-autoloads.el
Checking /var/folders/qy/zstv16390nlcs47kz8nff_mm0000gn/T/emacs-test-ysb37g-package/ustar-withsub-0.1...
Compiling /var/folders/qy/zstv16390nlcs47kz8nff_mm0000gn/T/emacs-test-ysb37g-package/ustar-withsub-0.1/ustar-withsub-autoloads.el...
Compiling /var/folders/qy/zstv16390nlcs47kz8nff_mm0000gn/T/emacs-test-ysb37g-package/ustar-withsub-0.1/ustar-withsub-pkg.el...
Compiling /var/folders/qy/zstv16390nlcs47kz8nff_mm0000gn/T/emacs-test-ysb37g-package/ustar-withsub-0.1/ustar-withsub.el...
Checking /var/folders/qy/zstv16390nlcs47kz8nff_mm0000gn/T/emacs-test-ysb37g-package/ustar-withsub-0.1/ustar-sub...
Done (Total of 1 file compiled, 2 skipped)
Setting `package-selected-packages' temporarily since "emacs -q" would overwrite customizations
Setting `package-selected-packages' temporarily since "emacs -q" would overwrite customizations
Package `ustar-withsub-0.1' deleted.
Parsing tar file... 
Parsing tar file...done
Parsing tar file... 
Parsing tar file...done
Test package-test-bug58367 backtrace:
  tar-header-name(nil)
  package-tar-file-info()
  package-install-from-buffer()
  package-install-file("/Users/mattias/emacs/test/lisp/emacs-lisp/pack
  (let* ((pkg-el "v7-withsub-0.1.tar") (source-file (expand-file-name 
  (progn nil (package-initialize) (let* ((pkg-el "ustar-withsub-0.1.ta
  (unwind-protect (progn nil (package-initialize) (let* ((pkg-el "usta
  (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn
  (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current
  (progn (fset 'y-or-n-p vnew) (fset 'yes-or-no-p vnew) (let ((temp-bu
  (unwind-protect (progn (fset 'y-or-n-p vnew) (fset 'yes-or-no-p vnew
  (let* ((vnew #'(lambda (&rest _) t)) (vnew #'(lambda (&rest _) t)) (
  (progn (cd (let* ((testfile (and t "/Users/mattias/emacs/test/lisp/e
  (unwind-protect (progn (cd (let* ((testfile (and t "/Users/mattias/e
  (let* ((process-environment (cons (format "HOME=%s" package-test-use
  (progn (let* ((process-environment (cons (format "HOME=%s" package-t
  (unwind-protect (progn (let* ((process-environment (cons (format "HO
  (let* ((coding-system-for-write nil) (temp-file (file-name-as-direct
  (closure (tar-parse-info t) nil (let* ((coding-system-for-write nil)
  ert--run-test-internal(#s(ert--test-execution-info :test ... :result
  ert-run-test(#s(ert-test :name package-test-bug58367 :documentation 
  ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
  ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
  ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
  ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
  eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
  command-line-1(("-L" ":../../emacs/test" "-l" "ert" "-l" "lisp/emacs
  command-line()
  normal-top-level()
Test package-test-bug58367 condition:
    (wrong-type-argument arrayp nil)
   FAILED   1/37  package-test-bug58367 (0.168213 sec) at ../../emacs/test/lisp/emacs-lisp/package-tests.el:288

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

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Thu, 27 Jul 2023 17:02:28 +0200
> Cc: Devon Sean McCullough <emacs-hacker2017@jovi.net>,
>  64686@debbugs.gnu.org,
>  Stefan Monnier <monnier@iro.umontreal.ca>
> 
> The change (2dc5f17c3e) causes packages-tests to fail; see attached log.
> This is on macOS with an out-of-tree build.

Thanks, should be fixed now.

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Mattias Engdegård]

27 juli 2023 kl. 18.19 skrev Eli Zaretskii <eliz@gnu.org>:

> Thanks, should be fixed now.

Confirmed, thank you.

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii [2023-07-27 11:38 +0300] wrote:
> Should be fixed now on the master branch.

Thanks, but doesn't tar-parse-pax-extended-header mutate
tar-attr-vector, which is initialised with a constant vector?

E.g. (aset [nil] 0 nil) would normally result in:
Warning: ‘aset’ on constant vector (arg 1)

Is this a welcome change?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tar.diff --]
[-- Type: text/x-diff, Size: 771 bytes --]

diff --git a/lisp/tar-mode.el b/lisp/tar-mode.el
index e4ea95343e0..2b6329fecc3 100644
--- a/lisp/tar-mode.el
+++ b/lisp/tar-mode.el
@@ -255,15 +255,14 @@ pax-decode-string
       (decode-coding-string str coding)
     str))
 
-(defvar tar-attr-vector '[nil nil nil nil nil nil nil nil])
+(defvar tar-attr-vector (make-vector 8 nil))
 (defun tar-parse-pax-extended-header (pos)
   "Parse a pax external header of a Posix-format tar file."
   (let ((end (+ pos 512))
         (result tar-attr-vector)
         (coding 'utf-8-unix)
         attr value record-len value-len)
-    (dotimes (i 8)
-      (aset result i nil))
+    (fillarray result nil)
     (goto-char pos)
     (while (and (< pos end)
                 (re-search-forward pax-extended-attribute-record-regexp

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


Thanks,
-- 
Basil

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Mattias Engdegård]

> Thanks, but doesn't tar-parse-pax-extended-header mutate tar-attr-vector, which is initialised with a constant vector?

Good catch. Little chance of detecting this one statically I'm afraid.

The patch is fine, but maybe we should return a fresh vector each time instead of reusing a single global object.

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: Devon Sean McCullough <emacs-hacker2017@jovi.net>,
>   64686@debbugs.gnu.org,  Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Fri, 28 Jul 2023 12:04:27 +0200
> 
> Thanks, but doesn't tar-parse-pax-extended-header mutate
> tar-attr-vector, which is initialised with a constant vector?
> 
> E.g. (aset [nil] 0 nil) would normally result in:
> Warning: ‘aset’ on constant vector (arg 1)
> 
> Is this a welcome change?

Yes, thanks.

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Fri, 28 Jul 2023 13:47:39 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  Devon Sean McCullough <emacs-hacker2017@jovi.net>,
>  64686@debbugs.gnu.org,
>  Stefan Monnier <monnier@iro.umontreal.ca>
> 
> maybe we should return a fresh vector each time instead of reusing a single global object.

I had that in the prototype, but decided I didn't like too much
consing this would produce.

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii [2023-07-28 15:21 +0300] wrote:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: Devon Sean McCullough <emacs-hacker2017@jovi.net>,
>>   64686@debbugs.gnu.org,  Stefan Monnier <monnier@iro.umontreal.ca>
>> Date: Fri, 28 Jul 2023 12:04:27 +0200
>> 
>> Thanks, but doesn't tar-parse-pax-extended-header mutate
>> tar-attr-vector, which is initialised with a constant vector?
>> 
>> E.g. (aset [nil] 0 nil) would normally result in:
>> Warning: ‘aset’ on constant vector (arg 1)
>> 
>> Is this a welcome change?
>
> Yes, thanks.

Thanks, installed:

; Avoid recent tar-mode.el constant mutation
6479e8758d9 2023-07-28 18:36:14 +0200
https://git.sv.gnu.org/cgit/emacs.git/commit/?id=6479e8758d9

-- 
Basil

bug#64686: 30.0.50; Tar mode @PaxHeader bug

[Eli Zaretskii]

> Cc: 64686@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Thu, 27 Jul 2023 11:38:17 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Mon, 17 Jul 2023 14:31:39 -0400
> > From: Devon Sean McCullough <emacs-hacker2017@jovi.net>
> > 
> > $ wget 
> > https://f-droid.org/repo/com.simplemobiletools.voicerecorder_36_src.tar.gz
> > $ emacs --no-init-file --no-site-file 
> > com.simplemobiletools.voicerecorder_36_src.tar.gz
> > 
> > should show something like the output of
> > 
> > $ tar -tvz < com.simplemobiletools.voicerecorder_36_src.tar.gz
> > 
> > instead of spurious ././@PaxHeader lines
> > interleaved with truncated long file names.
> 
> Thanks.  In a nutshell, tar-mode didn't support Posix-standard pax
> extended headers.  Should be fixed now on the master branch.
> 
> P.S. Thanks to Stefan who found a subtle bug in the prototype of this
> fix.

No further comments, so I assume the issue can be closed now.