[Midnight Commander] #4488: file corruption when copying from iso9660 with bad rrversion
Ticket System
--------------------------------+------------------------------
Reporter: ge0rg | Type: defect
Status: new | Priority: major
Milestone: Future Releases | Component: mc-vfs
Version: master | Keywords:
Blocked By: | Blocking:
Branch state: no branch | Votes for changeset:
--------------------------------+------------------------------
When opening certain ISO files with mc, I get an error pop-up on the first
open, which can be dismissed:
[[Image(https://op-co.de/tmp/bad-rrversion.png)]]
However, when copying individual files from such an ISO to another
directory using [F5], mc will silently attach the same string ("`**BAD
RRVERSION (0)`") to all copied files, making them 20 bytes larger than the
source, and corrupting all file formats that expect their header at the
end.
This looks related to #4326.
The only ISO I have seen this happen on yet is a proprietary driver CD
from Samsung from 2014, to which I can share a link privately on request.
Tested on Debian-provided mc, mc -V:
{{{
GNU Midnight Commander 4.8.29
Built with GLib 2.74.5
Built with S-Lang 2.3.3 with terminfo database
Built with libssh2 1.10.0
With builtin Editor and Aspell support
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
With ext2fs attributes support
Virtual File Systems:
cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, sftpfs, fish
Data types:
char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;
}}}
mc --configure-options:
{{{
'--build=x86_64-linux-gnu' '--prefix=/usr' '--
includedir=${prefix}/include' '--mandir=${prefix}/share/man' '--
infodir=${prefix}/share/info' '--sysconfdir=/etc' '--localstatedir=/var'
'--disable-option-checking' '--libdir=${prefix}/lib/x86_64-linux-gnu' '--
runstatedir=/run' '--disable-maintainer-mode' '--disable-dependency-
tracking' 'AWK=awk' 'X11_WWW=x-www-browser' '--libexecdir=/usr/lib' '--
with-x' '--with-screen=slang' '--disable-rpath' '--disable-static'
'--disable-silent-rules' '--enable-aspell' '--enable-vfs-sftp' '--enable-
vfs-undelfs' '--enable-tests' 'build_alias=x86_64-linux-gnu' 'CFLAGS=-g
-O2 -ffile-prefix-map=/build/mc-3Uz4Lz/mc-4.8.29=. -fstack-protector-
strong -Wformat -Werror=format-security' 'LDFLAGS=-Wl,-z,relro -Wl,-z,now
-Wl,--as-needed' 'CPPFLAGS=-Wdate-time -D_FORTIFY_SOURCE=2'
}}}
--
Ticket URL: <http://www.midnight-commander.org/ticket/4488>
Midnight Commander <https://midnight-commander.org>
Midnight Development Center
Ticket System
Reporter: ge0rg | Owner:
Type: defect | Status: new
Priority: major | Milestone: Future Releases
Component: mc-vfs | Version: master
Blocked By: | Blocking:
Branch state: no branch | Votes for changeset:
Comment (by zaytsev):
Why do you think it relates to #4326 ? In that ticket it was showing the
message, because `isoinfo` detection wasn't working correctly, however in
your case it seems that `isoinfo` is present, but cannot parse a specific
ISO file.
We don't need the CD, but we need the output of isoinfo and better still
test data like here:
https://github.com/MidnightCommander/mc/tree/master/tests/src/vfs/extfs
/helpers-list/data
So see where the RRVERSION comes from when parsing file list by `isoinfo`:
https://github.com/MidnightCommander/mc/blob/master/src/vfs/extfs/helpers/iso9660.in
--
Ticket URL: <http://www.midnight-commander.org/ticket/4488#comment:1>
Ticket System
Reporter: ge0rg | Owner:
Type: defect | Status: new
Priority: major | Milestone: Future Releases
Component: mc-vfs | Version: master
Blocked By: | Blocking:
Branch state: no branch | Votes for changeset:
Comment (by ge0rg):
Yes, `isoinfo` is present, but emits the warning when passed `-R` (I was
erroneously assuming that the warning is output to stderr, but it's
actually on stdout).
On the other hand, `isoinfo -d` reports "NO Rock Ridge present" for this
disk, so maybe the
[https://github.com/MidnightCommander/mc/blob/master/src/vfs/extfs/helpers/iso9660.in#L117
unconditional adding of -R in line 117] is the actual bug?
{{{
$ isoinfo -i nx-mini-software.iso
$ isoinfo -i nx-mini-software.iso -R
**BAD RRVERSION (0)
$ isoinfo -d -R -i nx-mini-software.iso
CD-ROM is in ISO 9660 format
System id:
Volume id: SAMSUNG
Volume set id:
Publisher id:
Data preparer id:
Application id: NERO BURNING ROM
Copyright File id:
Abstract File id:
Bibliographic File id:
Volume set size is: 1
Volume set sequence number is: 1
Logical block size is: 2048
Volume size is: 152704
Joliet with UCS level 3 found
NO Rock Ridge present
$ isoinfo -i nx-mini-software.iso -R > /dev/null
$ isoinfo -R -J -f -i nx-mini-software.iso
**BAD RRVERSION (0)
/Autorun.inf;1
/Mac
/Manual
/Windows
[...snipped...]
$ isoinfo -J -i nx-mini-software.iso -x
'/Windows/i-Launcher/i-Launcher_setup.exe;1' |grep RRVERSION
$ isoinfo -R -J -i nx-mini-software.iso -x
'/Windows/i-Launcher/i-Launcher_setup.exe;1' |grep RRVERSION
Binary file (standard input) matches
$
}}}
--
Ticket URL: <http://www.midnight-commander.org/ticket/4488#comment:2>
Ticket System
Reporter: ge0rg | Owner:
Type: defect | Status: new
Priority: major | Milestone: Future Releases
Component: mc-vfs | Version: master
Blocked By: | Blocking:
Branch state: no branch | Votes for changeset:
Comment (by zaytsev):
It was added in #4326, but I don't know why.
Maybe you are right and the "correct" solution would be to do something
like
{{{
if [ $(echo "$ISOINFO_D_I" | grep "NO Rock Ridge" | wc -l) = 0 ] ; then
ISOINFO="$ISOINFO -R"
fi
}}}
I would very much like to have test data for this before making further
changes to the script though :(
https://github.com/MidnightCommander/mc/tree/master/tests/src/vfs/extfs
/helpers-list/data
--
Ticket URL: <http://www.midnight-commander.org/ticket/4488#comment:3>
Ticket System
Reporter: ge0rg | Owner:
Type: defect | Status: new
Priority: major | Milestone: Future Releases
Component: mc-vfs | Version: master
Blocked By: | Blocking:
Branch state: no branch | Votes for changeset:
Comment (by ge0rg):
I have looked into the test data directory, but I don't know enough about
the required data structure to provide useful files. My naive hope was
that the above paste would be sufficient to at least generate an RR test
case.
Do you need input/output pairs for simulating `isoinfo` calls? With which
parameters? Or do you need input files to pass into `isoinfo`? How are
different test cases separated?
I suppose I could create something based on my "broken" ISO and maybe also
on another good one, if you can provide a howto for creating test cases ;)
--
Ticket URL: <http://www.midnight-commander.org/ticket/4488#comment:4>
Ticket System
Reporter: ge0rg | Owner:
Type: defect | Status: new
Priority: major | Milestone: Future Releases
Component: mc-vfs | Version: master
Blocked By: | Blocking:
Branch state: no branch | Votes for changeset:
Comment (by zaytsev):
We need output example of isoinfo and correct output of the helper on the
basis of this input. You can have a look how to add tests for example in
this commit:
https://github.com/MidnightCommander/mc/commit/d4ec872f37afa394e5aec9879bbf6d4d80a1964a
Just browse the directory on GitHub to see other commits which might
provide good examples:
https://github.com/MidnightCommander/mc/tree/master/tests/src/vfs/extfs
/helpers-list/data
The point is that if you adjust the script blindly the possibility to
break everything is very high. If you have example output of the backing
tool and desired output of the script, you can make sure that it's not
breaking like in #4326 when fixing an unrelated topic.
--
Ticket URL: <http://www.midnight-commander.org/ticket/4488#comment:5>