Re: [PATCH v2] tests: Add pathological iso9660 filesystem tests

[Glenn Washburn]

On Fri, 10 Feb 2023 11:44:57 +0100
"Thomas Schmitt" <scdbackup@gmx.net> wrote:

> Hi,
> 
> On Wed,  8 Feb 2023 23:52:17 -0600 Glenn Washburn
> <development@efficientek.com> wrote:
> > These are not added to grub-fs-tester because they are not
> > generated and none of the filesystem tests are run on these isos.
> > The test is to run the command "ls /" on the iso, and a failure is
> > determined if the command times out, has non-zero return value or
> > has any output.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> > v2: Remove temp directory containing extracted ISOs
> > ---
> >  tests/iso9660_ce_loop.iso.gz  | Bin 0 -> 897 bytes
> >  tests/iso9660_ce_loop2.iso.gz | Bin 0 -> 904 bytes
> >  tests/iso9660_test.in         |  12 +++++++++++-
> >  3 files changed, 11 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/iso9660_ce_loop.iso.gz
> >  create mode 100644 tests/iso9660_ce_loop2.iso.gz
> >
> > diff --git a/tests/iso9660_ce_loop.iso.gz
> > b/tests/iso9660_ce_loop.iso.gz new file mode 100644
> > index
> > 0000000000000000000000000000000000000000..9c53c569b8e5c2441273d49f4c8e13e4b3303c24
> > GIT binary patch literal 897
> > zcmb2|=3ofBusfN7`Rxt={M#uq3<<mEx`p{35L_s?rt`5_&ho@gi=FE>N2(O2EDtVk
> > zV!mF;l$o*IlVjfrJNEhM(>tw&wEQkz$h$g?`Kr;{^NWwz@Be?_yzkL<P9_HYK;caP
> > zBMZ*vjOI-%PkDY@P!aue*Zzaq4`m;3TYuF0MM=}%&};V&Ru<M5?hszGIz*~OY@x6X
> > zcYbM}Jd5?~Z?n_>ZnR$cYS&()@00TO-|oL;;k(=OmM{C=iVM<ee|KNpVUu&3zyI~i
> > zRp+eIuf%xF7tgP@mEZA=`Q1@Zn;v%0TmFwNc1#i6|FFnSBBxq3=X&U$oWoNsY<K@>
> > z#_wqSvIkrf>qL%wO1MZ*@eTFc#D4Vlsq24hKSos?-|;s4BcIjWg#4*n%M0S}#LK*6
> > zd8sv(ZLaGJmh*?A=iX*Ncy8C<Ws&u7{8sJCTYqNnlH2dUEjIDHoHwcLvZJ}&mp0G;
> > zc_w=Nw?7?zT(xV}=5vy!;+LKuy&ZR+ecj)N>Ovv@$BvV}UAExUOFfdmV~!x(?Ksxk
> > zrK0w?3sn8izx&T{;OocR$I{=Vee?YBJpSODC#ye%Zd);Fuhnh6m|Lc2+~eEk&)ZsW
> > z`{Zcdjp)!<YqS4v=CjY4<^1D6+ke{~`&Sj!r;C=0vCjRwIq#6P`rZ7=PnqIv;-A>o
> > zZ<Krd^qtb1eKE)XAIp%|KYaaQWR}%VNj87+(=qE)cfC=W^KSj6-?P`VGVIZJ{i}6n
> > z_J6@^rIx?#B>5)i{M*p+O6$Mgs?&?vT|@kyyUw*gv+V89HS3;*#`TJ;_n%t4sB_li
> > zn`>4*zqn%6;>fF0)Gw}(5fA*lW2X4z6`E={OX8lL&523z6?}TNWZu)yFIMDy$|$R`
> > z(3{=vx;N*}nXWxDQ!CB%S7%-cIbIr9vUpqjBd4QfC${Yl+-Wg)+hh&($iEg<A-(<T
> > zu2=lj^TiF<hsmxxuC+WjYtx%;vwQ9H^H1$Iulr@W<KjClzs(;rcOTrj^Y>$Y|9O9;
> > z{>Z9lzF+YF?B9bQs^+9VK7Z(e&Ca*$l^EU!?x^|p?Y})kjQVNo`YdodlHa_%`u03V
> > Tj6@S-|DS2eE8$QE1||jom5H3-
> >
> > literal 0
> > HcmV?d00001
> >
> > diff --git a/tests/iso9660_ce_loop2.iso.gz
> > b/tests/iso9660_ce_loop2.iso.gz new file mode 100644
> > index
> > 0000000000000000000000000000000000000000..f59e676475cfc8376256ba65047451b5ca969e5a
> > GIT binary patch literal 904
> > zcmb2|=3rP?urHZ``Rxt={M#uq3<<mEx`p{35L_s`CiA%N4Zp)eg+;;Xn>`+#@LT?l
> > zg?-)QBTE*}RZ}c{9Q>yELuJ{_9m)%n)Op{A<yr=u*kac&^#9Ml`ui4L@#-85_(4NK
> > ziNtYF2^VRR(nz1p>_2y(y8f5<9>;sh?Uh^KFSyM4EiAJ7cly!4-wLcf!dIHS*s@UA
> > zhCja~?;X4K>u<Bu{%*9^TfJ(h(f3Jt`)~JOvhdyQdCQmmZp8&@wZFSBF0sfs#oxE;
> > z=Bjg6=`r)Y=Zof7+sf~F#{BN6r%eyL=Pm!o7CR;g?thx(CXrJunsYt%PtM_`7M6Se
> > zGvjwOe%S++4__GcO*ZH-(|EOMMjHR8@~P{8|9;f_VP4_v@ZWq^a})BXZrxrVb|;^I
> > zdvnQDuVz1KtAn3ExIKSXw_$VM{>*FgyB}{_HA}l#|K+#ce>Hk9Z<)R1ZpnpS_PvRh
> > z_DlPAH81<~V8yJpuhtYF`?~W><i~B-KR0IEJ&2og<Z#7=C4Wu&nz<!EM(d@u9NZRu
> > zuq^i2d9#?V$(MiIGZd8m+4NCz_q=+cfB)PK?Dq-pJ9kU#=IhIE54FY~PWqGj<7MV=
> > z^TQhZMZbkjt$KZ{e(mA!v!4n4+kLQ}``!9gckV|Xox5#8@#`(KKin$(K09USf$93U
> > z_K53WPu_DY_wc*brGL_UmVGp;5uLLv?egoy4{v@Hs^?t4q*uOo+09Lx3(C@`?%Vuv
> > z62pOdz4dYJ)!p%jS7|-FuX;FflCIpdz|g*Zy{jI5aFi3>>>AfAm$vy?P2AM(q7c7Z
> > zliiOd#<_pJSQ8i4oe>juanG9W#~Cp`pH|ex=zq!#d44%(n}*xhnn~%Ua=oW+DoN`u
> > zI=o2E_4DFp&p`V#t5Y9W&Fk+fG(08dYT5nj(+a7lcUQbrv=84Xx#?wI%$DVw(}a$i
> > ztd?`{KXvk=N&l>uJ+oYOPKCC=-q;m6d6wSpw`r5l&z}A9-B$T}u63rnyC&xpUCJp~
> > z_x|69FDEC@Z?=D|^7ifb{rl`C;(9mde2SEqqq_ZXD9eL-5xx0)|NiG^2tWDh-27Xh
> > c^m5?%o16ExrDG<Y@bmX~KG-skfq{tu09s$LV*mgE
> >
> > literal 0
> > HcmV?d00001
> >
> > diff --git a/tests/iso9660_test.in b/tests/iso9660_test.in
> > index ed0a5bf8d4..0ec28bd066 100644
> > --- a/tests/iso9660_test.in
> > +++ b/tests/iso9660_test.in
> > @@ -12,4 +12,14 @@ fi
> >  "@builddir@/grub-fs-tester" rockridge_joliet
> >  "@builddir@/grub-fs-tester" joliet_1999
> >  "@builddir@/grub-fs-tester" rockridge_1999
> > -"@builddir@/grub-fs-tester" rockridge_joliet_1999
> > \ No newline at end of file
> > +"@builddir@/grub-fs-tester" rockridge_joliet_1999
> > +
> > +for fs in iso9660_ce_loop iso9660_ce_loop2; do
> > +  tempdir=`mktemp -d "${TMPDIR:-/tmp}/${0##*/}.$(date
> > '+%Y%m%d%H%M%S%N').${fs}.XXX"` ||
> > +    { echo "Failed to make temporary directory"; exit 99; }
> > +  gunzip <"$srcdir"/tests/${fs}.iso.gz >"${tempdir}/${fs}.iso"
> > +  output=$(LC_ALL=C timeout -s KILL "60" \
> > +    "@builddir@/grub-fstest" "${tempdir}/${fs}.iso" ls / ) ||
> > ret=$?
> > +  rm -rf "$tempdir"
> > +  test "${ret:-0}" -eq 0 -a -z "$output"
> > +done
> > --
> > 2.34.1
> 
> Tested-by: Thomas Schmitt <scdbackup@gmx.net>
> 
> 
> I tested with the current state of
>   https://git.savannah.gnu.org/git/grub.git
> by
>   make check
> with the expected results.
> 
> With Lidong Chen's patch 5 v3 still in effect:
>   PASS: iso9660_test
> 
> With the loop breaker of that patch disabled:
>   FAIL: iso9660_test
> after about 60 seconds.
> 
> With loop breaker enabled again, but the special handling of CE at the
> start of a continuation area disabled:
>   FAIL: iso9660_test
> after some time but less than 60 seconds.
> (I assume the failure happens because the ls of ce_loop.iso yielded a
> file name.)
> 
> No files matching '*.iso' were found remaining in the repo tree
> afterwards.

Thank you for doing this thorough testing.
 -------------------------------------------------------------------------
> 
> But the message in test-suite.log is very sparse:
> 
>   FAIL iso9660_test (exit status: 1)
> 
> One could come to the impression that one of the logged xorriso runs
> would be to blame. Those runs emit lots of warnings because of specs
> violating Volume Id and symbolic links in Joliet.
> 
> Maybe a message before the loop in tests/iso9660_test.in could help
> the puzzled user. Like:
>   Testing for proper recognition of CE loops ...

This is a good idea, thanks for the suggestion. I'll make a v3 with
better output.

Glenn