beetbox / pyacoustid

Python bindings for Chromaprint acoustic fingerprinting and the Acoustid Web service
MIT License
325 stars 66 forks source link

libchromaprint.dll not found when using Python 3.8 on Windows #54

Closed valpogus closed 4 years ago

valpogus commented 4 years ago

The shared library 'libchromaprint.dll' is not found when using Python 3.8 on Windows even when the file is in a directory in the PATH. It is found when using Python 3.7 though.

Python 3.7.4 (tags/v3.7.4:e09359112e, Jul  8 2019, 20:34:20) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import chromaprint
Python 3.8.0 (tags/v3.8.0:fa919fd, Oct 14 2019, 19:37:50) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import chromaprint
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python3.8\lib\site-packages\chromaprint.py", line 39, in <module>
    raise ImportError("couldn't find libchromaprint")
ImportError: couldn't find libchromaprint

The error possibly has to do with the way the DLL is loaded in chromaprint.py and the following change to ctypes in Python 3.8: https://docs.python.org/3/whatsnew/3.8.html#ctypes

On Windows, CDLL and subclasses now accept a winmode parameter to specify flags for the underlying LoadLibraryEx call. The default flags are set to only load DLL dependencies from trusted locations, including the path where the DLL is stored (if a full or partial path is used to load the initial DLL) and paths added by add_dll_directory().

Python 3.7.4 (tags/v3.7.4:e09359112e, Jul  8 2019, 20:34:20) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import ctypes
>>> _libchromaprint = ctypes.cdll.LoadLibrary('libchromaprint.dll')
Python 3.8.0 (tags/v3.8.0:fa919fd, Oct 14 2019, 19:37:50) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import ctypes
>>> _libchromaprint = ctypes.cdll.LoadLibrary('libchromaprint.dll')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python3.8\lib\ctypes\__init__.py", line 447, in LoadLibrary
    return self._dlltype(name)
  File "C:\Python3.8\lib\ctypes\__init__.py", line 369, in __init__
    self._handle = _dlopen(self._name, mode)
FileNotFoundError: Could not find module 'libchromaprint.dll'. Try using the full path with constructor syntax.

I've thought about a couple of possible solutions, but I don't know, which one is right (if any)

https://docs.python.org/3/library/ctypes.html?highlight=winmode#ctypes.CDLL https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa

The winmode parameter is used on Windows to specify how the library is loaded (since mode is ignored). It takes any value that is valid for the Win32 API LoadLibraryEx flags parameter. When omitted, the default is to use the flags that result in the most secure DLL load to avoiding issues such as DLL hijacking. Passing the full path to the DLL is the safest way to ensure the correct library and dependencies are loaded.

def _load_library(name):
    try:
        return ctypes.CDLL(name, winmode=0x00000008)
    except TypeError:
        return ctypes.CDLL(name)

for name in _guess_lib_name():
    try:
        _libchromaprint = _load_library(name)
        break
    except OSError:
        pass
else:
    raise ImportError("couldn't find libchromaprint")

https://docs.python.org/3/library/ctypes.html?highlight=winmode#finding-shared-libraries

import ctypes.util

def _load_library(name):
    if sys.platform == 'win32':
        try:
            return ctypes.cdll.LoadLibrary(ctypes.util.find_library(name))
        except TypeError:
            raise OSError()
    else:
        return ctypes.cdll.LoadLibrary(name)

for name in _guess_lib_name():
    try:
        _libchromaprint = _load_library(name)
        break
    except OSError:
        pass
else:
    raise ImportError("couldn't find libchromaprint")
sampsyo commented 4 years ago

Interesting! I really don't know much about DLLs on Windows, but this caught my eye:

The default flags are set to only load DLL dependencies from trusted locations

Are there any locations that are trusted by default? If so, would it make sense to put the DLL there on your system?

Otherwise, if there are no "trusted locations" by default, maybe we should learn more about exactly what the security concerns are here. If just saying "search everywhere" or "use find_library" would undo the security this is meant to provide, maybe we need another option—such as an environment variable to indicate where the library is supposed to exist.

valpogus commented 4 years ago

A quick search of my site-packages shows that many modules do use ctypes.util.find_library() (audioread, cffi, cairocffi, fdb, python-magic, send2trash...) and none uses winmode. I don't know about security, but find_library() seems to be at least the de facto standard.

Other libraries like pymediainfo ship the shared library inside the Windows wheel and then look for the library file in the installation directory.

sampsyo commented 4 years ago

Cool! It does seem like find_library is pretty low on the concern scale. TBH I still don't quite understand what the "DLL hijacking" problem was with the old behavior, but find_library seems reasonable enough?

Anyway, yes, that does seem like the right fix. Any chance you could open a PR?

valpogus commented 4 years ago

OK, I'll do that

valpogus commented 4 years ago

Solved with https://github.com/beetbox/pyacoustid/commit/d8adb6642e90c7b2c775c1b706aad6575afe78b1

valpogus commented 4 years ago

@sampsyo any plans to release the latest version on PyPi?

sampsyo commented 4 years ago

Yep! Soon.