On Sun, Dec 25, 2022 at 08:11:02PM +0800, Qu Wenruo wrote:
>
>
> On 2022/12/25 19:51, Zorro Lang wrote:
> > On Fri, Dec 23, 2022 at 10:56:42AM +0800, Qu Wenruo wrote:
> > > Test case btrfs/154 is still using python2 script, which is already EOL.
> > > Some rolling distros like Archlinux is no longer providing python2
> > > package anymore.
> > >
> > > This means btrfs/154 will be harder and harder to run.
> > >
> > > To fix the problem, migreate the python script to python3, this involves
> > > the following changes:
> > >
> > > - Change common/config to use python3
> > > - Strong type conversion between string and bytes
> > > This means, anything involved in the forged bytes has to be bytes.
> > >
> > > And there is no safe way to convert forged bytes into string, unlike
> > > python2.
> > > I guess that's why the author went python2 in the first place.
> > >
> > > Thankfully os.rename() still accepts forged bytes.
> > >
> > > - Use bytes specific checks for invalid chars.
> > >
> > > The updated script can still cause the needed conflicts, can be verified
> > > through "btrfs ins dump-tree" command.
> > >
> > > Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> > > ---
> > > common/config | 2 +-
> > > src/btrfs_crc32c_forged_name.py | 22 ++++++++++++++++------
> > > tests/btrfs/154 | 4 ++--
> > > 3 files changed, 19 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/common/config b/common/config
> > > index b2802e5e..e2aba5a9 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -212,7 +212,7 @@ export NFS4_SETFACL_PROG="$(type -P nfs4_setfacl)"
> > > export NFS4_GETFACL_PROG="$(type -P nfs4_getfacl)"
> > > export UBIUPDATEVOL_PROG="$(type -P ubiupdatevol)"
> > > export THIN_CHECK_PROG="$(type -P thin_check)"
> > > -export PYTHON2_PROG="$(type -P python2)"
> > > +export PYTHON3_PROG="$(type -P python3)"
> >
> > How about:
> >
> > export PYTHON_PROG="$(type -P python)"
> > export PYTHON2_PROG="$(type -P python2)"
> > export PYTHON3_PROG="$(type -P python3)"
> >
> > maybe someone still need python2, or maybe someone doesn't care the version.
>
> Even for distros which completely get rid of python2 and make python3
> default, there is still python3.
> And python is just a softlink to python3, which further links to the real
> python
>
> $ type -P python2
> $ type -P python3
> /usr/bin/python3
> $ type -P python
> /usr/bin/python
>
> $ ls -alh /usr/bin/python
> lrwxrwxrwx 1 root root 7 Nov 1 22:18 /usr/bin/python -> python3
> $ ls -alh /usr/bin/python3
> lrwxrwxrwx 1 root root 10 Nov 1 22:18 /usr/bin/python3 -> python3.10
>
> Secondly, for this particular case, we must distinguish python2 and python3,
> especially due to the strong requirement for string encoding.
>
>
> So to your PYTHON_PROG usage, no, it's not good at all, it's going to be
> distro dependent and will be a disaster if some one is using PYTHON_PROG.
>
> For PYTHON2_PROG, there is no usage of it any more, thus I see now reason to
> define it.
OK, due to only btrfs/154 uses this PYTHON?_PROG thing now, we can talk about
that later. This patch looks good to me.
Reviewed-by: Zorro Lang <zlang@xxxxxxxxxx>
>
> Thanks,
> Qu
> >
> > Thanks,
> > Zorro
> >
> > > export SQLITE3_PROG="$(type -P sqlite3)"
> > > export TIMEOUT_PROG="$(type -P timeout)"
> > > export SETCAP_PROG="$(type -P setcap)"
> > > diff --git a/src/btrfs_crc32c_forged_name.py b/src/btrfs_crc32c_forged_name.py
> > > index 6c08fcb7..d29bbb70 100755
> > > --- a/src/btrfs_crc32c_forged_name.py
> > > +++ b/src/btrfs_crc32c_forged_name.py
> > > @@ -59,9 +59,10 @@ class CRC32(object):
> > > # deduce the 4 bytes we need to insert
> > > for c in struct.pack('<L',fwd_crc)[::-1]:
> > > bkd_crc = ((bkd_crc << 8) & 0xffffffff) ^ self.reverse[bkd_crc >> 24]
> > > - bkd_crc ^= ord(c)
> > > + bkd_crc ^= c
> > > - res = s[:pos] + struct.pack('<L', bkd_crc) + s[pos:]
> > > + res = bytes(s[:pos], 'ascii') + struct.pack('<L', bkd_crc) + \
> > > + bytes(s[pos:], 'ascii')
> > > return res
> > > def parse_args(self):
> > > @@ -72,6 +73,12 @@ class CRC32(object):
> > > help="number of forged names to create")
> > > return parser.parse_args()
> > > +def has_invalid_chars(result: bytes):
> > > + for i in result:
> > > + if i == 0 or i == int.from_bytes(b'/', byteorder="little"):
> > > + return True
> > > + return False
> > > +
> > > if __name__=='__main__':
> > > crc = CRC32()
> > > @@ -80,12 +87,15 @@ if __name__=='__main__':
> > > args = crc.parse_args()
> > > dirpath=args.dir
> > > while count < args.count :
> > > - origname = os.urandom (89).encode ("hex")[:-1].strip ("\x00")
> > > + origname = os.urandom (89).hex()[:-1].strip ("\x00")
> > > forgename = crc.forge(wanted_crc, origname, 4)
> > > - if ("/" not in forgename) and ("\x00" not in forgename):
> > > + if not has_invalid_chars(forgename):
> > > srcpath=dirpath + '/' + str(count)
> > > - dstpath=dirpath + '/' + forgename
> > > - file (srcpath, 'a').close()
> > > + # We have to convert all strings to bytes to concatenate the forged
> > > + # name (bytes).
> > > + # Thankfully os.rename() can accept bytes directly.
> > > + dstpath=bytes(dirpath, "ascii") + bytes('/', "ascii") + forgename
> > > + open(srcpath, mode="a").close()
> > > os.rename(srcpath, dstpath)
> > > os.system('btrfs fi sync %s' % (dirpath))
> > > count+=1;
> > > diff --git a/tests/btrfs/154 b/tests/btrfs/154
> > > index 240c504c..6be2d5f6 100755
> > > --- a/tests/btrfs/154
> > > +++ b/tests/btrfs/154
> > > @@ -21,7 +21,7 @@ _begin_fstest auto quick
> > > _supported_fs btrfs
> > > _require_scratch
> > > -_require_command $PYTHON2_PROG python2
> > > +_require_command $PYTHON3_PROG python3
> > > # Currently in btrfs the node/leaf size can not be smaller than the page
> > > # size (but it can be greater than the page size). So use the largest
> > > @@ -42,7 +42,7 @@ _scratch_mount
> > > # ...
> > > #
> > > -$PYTHON2_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310
> > > +$PYTHON3_PROG $here/src/btrfs_crc32c_forged_name.py -d $SCRATCH_MNT -c 310
> > > echo "Silence is golden"
> > > # success, all done
> > > --
> > > 2.39.0
> > >
> >
>
