fix: check for negative coordinates (#445)

* Add a test for tuples that have negative coordinates.

Specifically, a negative width or height.

Particularly, this seems like an easy case for a user to hit if
somebody forgets that the PIL tuple style uses top, left, RIGHT,
BOTTOM and not top, left, WIDTH, HEIGHT.

Ask how I know.

Test that we do handle that gracefully.

* Ensure that grab regions are valid

* Remove a now-obsolete test

I thought I'd seen a test for for invalid regions somewhere before...

Author: jholveck

src/mss/base.py

@@ -112,6 +112,13 @@ def grab(self, monitor: Monitor | tuple[int, int, int, int], /) -> ScreenShot:
                 "height": monitor[3] - monitor[1],
             }
 
+        if monitor["left"] < 0 or monitor["top"] < 0:
+            msg = f"Region has negative coordinates: {monitor!r}"
+            raise ScreenShotError(msg)
+        if monitor["width"] <= 0 or monitor["height"] <= 0:
+            msg = f"Region has zero or negative size: {monitor!r}"
+            raise ScreenShotError(msg)
+
         with lock:
             screenshot = self._grab_impl(monitor)
             if self.with_cursor and (cursor := self._cursor_impl()):

src/tests/test_gnu_linux.py

@@ -170,39 +170,6 @@ def test_unsupported_depth(backend: str) -> None:
         sct.grab(sct.monitors[1])
 
 
-def test_region_out_of_monitor_bounds(display: str, backend: str) -> None:
-    monitor = {"left": -30, "top": 0, "width": WIDTH, "height": HEIGHT}
-
-    with mss.mss(display=display, backend=backend, with_cursor=True) as sct:
-        # At one point, I had accidentally been reporting the resource ID as a CData object instead of the contained
-        # int.  This is to make sure I don't repeat that mistake.  That said, change this error regex if needed to keep
-        # up with formatting changes.
-        expected_err_re = (
-            r"(?is)"
-            r"Error of failed request:\s+(8|BadMatch)\b"
-            r".*Major opcode of failed request:\s+73\b"
-            r".*Resource id in failed request:\s+[0-9]"
-            r".*Serial number of failed request:\s+[0-9]"
-        )
-
-        with pytest.raises(ScreenShotError, match=expected_err_re) as exc:
-            sct.grab(monitor)
-
-        details = exc.value.details
-        assert details
-        assert isinstance(details, dict)
-        if backend in {"xgetimage", "xshmgetimage"} and mss.linux.xcb.LIB.errors is None:
-            pytest.xfail("Error strings in XCB backends are only available with the xcb-util-errors library.")
-        assert isinstance(details["error"], str)
-
-        errstr = str(exc.value)
-        assert "Match" in errstr  # Xlib: "BadMatch"; XCB: "Match"
-        assert "GetImage" in errstr  # Xlib: "X_GetImage"; XCB: "GetImage"
-
-        if backend == "xlib":
-            assert not mss.linux.xlib._ERROR
-
-
 def test__is_extension_enabled_unknown_name(display: str) -> None:
     with mss.mss(display=display, backend="xlib") as sct:
         assert isinstance(sct, mss.linux.xlib.MSS)  # For Mypy

src/tests/test_implementation.py

@@ -251,6 +251,31 @@ def test_grab_with_tuple(mss_impl: Callable[..., MSSBase]) -> None:
         assert im.rgb == im2.rgb
 
 
+def test_grab_with_invalid_tuple(mss_impl: Callable[..., MSSBase]) -> None:
+    with mss_impl() as sct:
+        # Remember that rect tuples are PIL-style: (left, top, right, bottom)
+
+        # Negative top
+        negative_box = (100, -100, 500, 500)
+        with pytest.raises(ScreenShotError):
+            sct.grab(negative_box)
+
+        # Negative left
+        negative_box = (-100, 100, 500, 500)
+        with pytest.raises(ScreenShotError):
+            sct.grab(negative_box)
+
+        # Negative width (but right > 0)
+        negative_box = (100, 100, 50, 500)
+        with pytest.raises(ScreenShotError):
+            sct.grab(negative_box)
+
+        # Negative height (but bottom > 0)
+        negative_box = (100, 100, 500, 50)
+        with pytest.raises(ScreenShotError):
+            sct.grab(negative_box)
+
+
 def test_grab_with_tuple_percents(mss_impl: Callable[..., MSSBase]) -> None:
     with mss_impl() as sct:
         monitor = sct.monitors[1]