BoboTiG / python-mss

An ultra fast cross-platform multiple screenshots module in pure Python using ctypes.
https://pypi.org/project/mss/
MIT License
987 stars 88 forks source link

linux: Add failure handling to XOpenDisplay() call #247

Closed mgorny closed 1 year ago

mgorny commented 1 year ago

Add an explicit check for the return value of XOpenDisplay(). This function does not seem to use X11 error handlers, and only returns NULL when it fails. Without an explicit check, mss ended up passing this NULL value to further calls and causing segfaults in libX11. Now it triggers an explicit exception instead.

Changes proposed in this PR

It is very important to keep up to date tests and documentation.

Is your code right?

BoboTiG commented 1 year ago

Can you also add a test to cover the new code 🙏🏻 ?

mgorny commented 1 year ago

Can you also add a test to cover the new code 🙏🏻 ?

Hmm, I'll try.

mgorny commented 1 year ago

I've done that by passing display=":INVALID". This is not perfect but I don't have a good idea how to make XOpenDisplay() fail without guessing display numbers that would invalid (but could be valid on the particular host, especially given how random the numbers given by Xvfb seem to be).

BoboTiG commented 1 year ago

Thanks @mgorny :)

mgorny commented 1 year ago

Thanks. Would you like me to try converting the tests into using PyVirtualDisplay or pytest-xvfb? FWICS the latter may be just what we need: https://pypi.org/project/pytest-xvfb/#features

mgorny commented 1 year ago

Ah, sorry, I see you've replied on #246. I have a minute right now, so I'll try.