LudovicRousseau / pyscard

pyscard smartcard library for python
http://pyscard.sourceforge.net/
GNU Lesser General Public License v2.1
403 stars 115 forks source link

Is `ulist` orderering important? #195

Closed kurtmckee closed 1 month ago

kurtmckee commented 2 months ago

I'm looking at the implementation of ulist and it looks very similar to Python sets (pyscard has existed long enough that it may predate when sets were introduced in Python).

Is the insertion and iteration ordering of items in ulist important? I didn't immediately find an answer while perusing the codebase.

LudovicRousseau commented 2 months ago

I nerver used ulist. This class is used only in src/smartcard/reader/ReaderGroups.py. Maybe it can be completely replaced by Python sets.

According to https://codesearch.debian.net/search?q=smartcard.ulist smartcard.ulist is it used only internally by PySCard.

kurtmckee commented 2 months ago

Okay, I'll take a look at migrating to Python sets. :+1:

kurtmckee commented 1 month ago

My question is answered. Closing.

LudovicRousseau commented 1 month ago

ulist on Windows is broken:

(base) C:\Users\Ludovic\Documents\pyscard-devel\src\smartcard\test\framework>python testcase_readergroups.py
EEFFF
======================================================================
ERROR: testcase_readergroup_add (__main__.testcase_readergroups.testcase_readergroup_add)
tests groups=groups+[newgroups]
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Ludovic\Documents\pyscard-devel\src\smartcard\test\framework\testcase_readergroups.py", line 51, in testcase_readergroup_add
    groups = groups + [self.pinpadgroup]
             ~~~~~~~^~~~~~~~~~~~~~~~~~~~
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 47, in __add__
    self.__appendother__(newother)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 114, in __appendother__
    self.__onadditem__(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 52, in __onadditem__
    self.addreadergroup(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\pcsc\PCSCReaderGroups.py", line 70, in addreadergroup
    innerreadergroups.addreadergroup(self, newgroup)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 73, in addreadergroup
    self += newgroup
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 52, in __iadd__
    self.__appendother__(newother)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 114, in __appendother__
    self.__onadditem__(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 52, in __onadditem__
    self.addreadergroup(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\pcsc\PCSCReaderGroups.py", line 70, in addreadergroup
    innerreadergroups.addreadergroup(self, newgroup)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 73, in addreadergroup
    self += newgroup
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 52, in __iadd__
    self.__appendother__(newother)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 114, in __appendother__
    self.__onadditem__(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 52, in __onadditem__
    self.addreadergroup(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\pcsc\PCSCReaderGroups.py", line 70, in addreadergroup
    innerreadergroups.addreadergroup(self, newgroup)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 73, in addreadergroup
    self += newgroup
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 52, in __iadd__
    self.__appendother__(newother)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 114, in __appendother__
    self.__onadditem__(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 52, in __onadditem__
    self.addreadergroup(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\pcsc\PCSCReaderGroups.py", line 70, in addreadergroup
    innerreadergroups.addreadergroup(self, newgroup)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 73, in addreadergroup
    self += newgroup
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 52, in __iadd__
    self.__appendother__(newother)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 114, in __appendother__
    self.__onadditem__(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 52, in __onadditem__
    self.addreadergroup(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\pcsc\PCSCReaderGroups.py", line 70, in addreadergroup
    innerreadergroups.addreadergroup(self, newgroup)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 73, in addreadergroup
    self += newgroup
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 52, in __iadd__

[...]

  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 52, in __onadditem__
    self.addreadergroup(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\pcsc\PCSCReaderGroups.py", line 70, in addreadergroup
    innerreadergroups.addreadergroup(self, newgroup)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 73, in addreadergroup
    self += newgroup
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 52, in __iadd__
    self.__appendother__(newother)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\ulist.py", line 114, in __appendother__
    self.__onadditem__(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\reader\ReaderGroups.py", line 52, in __onadditem__
    self.addreadergroup(item)
  File "C:\Users\Ludovic\miniconda3\Lib\site-packages\smartcard\pcsc\PCSCReaderGroups.py", line 60, in addreadergroup
    hresult, hcontext = SCardEstablishContext(SCARD_SCOPE_USER)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RecursionError: maximum recursion depth exceeded

I think we can safely replace it by something else.

kurtmckee commented 1 month ago

Agreed. I'm just getting back from vacation and need to refresh my brain about this, but I'm pretty sure that a ulist instance is returned by some methods or properties, and that ykman (the Yubikey command line tool) may be using that, so replacing ulist may require some delicacy or coordination.

I may be misremembering, though, since I haven't looked at a computer in a week. I'm still intending to work on this.

kurtmckee commented 1 month ago

Oh! And this is an issue with .addreadergroup(), and it can happen across all operating systems.

I noted this problem in the pathological tests for readergroups.