diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4c7697b..e05beec 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -33,6 +33,15 @@ jobs: run: | python3 -m pip install . + - name: Test snap params-cache regression (mock backend) + # CPython only: the mock test runs the C extension under LD_PRELOAD, + # which is not reliable under cpyext (PyPy). + if: ${{ !contains(matrix.python-version, 'pypy') }} + shell: bash + run: | + python3 -m pip install pytest + python3 -m pytest tests/test_snap_params_cache.py -v + - name: Lint shell: bash run: | diff --git a/CHANGES.rst b/CHANGES.rst index da20ba6..2b4288e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,23 @@ +Unreleased +---------- + +- _sane.c: + + - Fix `SaneDev.snap()` failing with `SANE_STATUS_INVAL` against backends + that return `SANE_STATUS_INVAL` on a second `sane_get_parameters()` call + after `sane_start` (e.g. Epson `epsonscan2`). The parameters retrieved by + `get_parameters()` are now cached on the `SaneDevObject` and reused by + `snap()`, instead of issuing a redundant `sane_get_parameters()` call. + (fixes #107) + +- tests: + + - Add a mock-backend regression test (`tests/mock/libsane_mock.c`) that + emulates a backend returning `SANE_STATUS_INVAL` on a repeated + `sane_get_parameters()` call, exercising snap() through the cached-params + path (depth-8, depth-1, and snap without a prior get_parameters). + + Version 2.9.2 ------------- diff --git a/_sane.c b/_sane.c index 2cda001..e5b6149 100644 --- a/_sane.c +++ b/_sane.c @@ -43,6 +43,8 @@ static PyObject *ErrorObject; typedef struct { PyObject_HEAD SANE_Handle h; + SANE_Parameters cached_params; + int params_valid; } SaneDevObject; static PyTypeObject SaneDev_Type; @@ -99,6 +101,9 @@ SaneDev_get_parameters(SaneDevObject *self, PyObject *args) if(st != SANE_STATUS_GOOD) return PySane_Error(st); + + self->cached_params = p; + self->params_valid = 1; switch(p.format) { case(SANE_FRAME_GRAY): format="gray"; break; @@ -143,6 +148,7 @@ SaneDev_start(SaneDevObject *self, PyObject *args) Py_END_ALLOW_THREADS if(st != SANE_STATUS_GOOD) return PySane_Error(st); + self->params_valid = 0; Py_INCREF(Py_None); return Py_None; } @@ -399,11 +405,23 @@ SaneDev_snap(SaneDevObject *self, PyObject *args) } RAISE_IF(self->h == NULL, "SaneDev object is closed"); - /* Get parameters, prepare buffers */ + /* Get parameters, prepare buffers. + Use the cached params from the most recent get_parameters() call. + The epsonscan2 backend returns INVAL on a second sane_get_parameters() + call after sane_start; the wrapper's get_parameters() already retrieved + and cached the correct params. If the cache is empty (snap called + without a prior get_parameters), fall back to sane_get_parameters. */ SANE_Parameters p = {}; - st = sane_get_parameters(self->h, &p); - if(st != SANE_STATUS_GOOD) - return PySane_Error(st); + if(self->params_valid) + { + p = self->cached_params; + } + else + { + st = sane_get_parameters(self->h, &p); + if(st != SANE_STATUS_GOOD) + return PySane_Error(st); + } RAISE_IF(p.depth != 1 && p.depth != 8 && p.depth != 16, "Bad pixel depth"); @@ -455,9 +473,12 @@ SaneDev_snap(SaneDevObject *self, PyObject *args) st = sane_start(self->h); if(st != SANE_STATUS_GOOD) break; + self->params_valid = 0; st = sane_get_parameters(self->h, &p); if(st != SANE_STATUS_GOOD) break; + self->cached_params = p; + self->params_valid = 1; /* Continue reading */ continue; } @@ -725,6 +746,7 @@ PySane_open(PyObject *self, PyObject *args) SaneDevObject *dev = PyObject_NEW(SaneDevObject, &SaneDev_Type); RAISE_IF(dev == NULL, "Failed to create SaneDev object"); dev->h = NULL; + dev->params_valid = 0; SANE_Status st; Py_BEGIN_ALLOW_THREADS diff --git a/tests/mock/libsane_mock.c b/tests/mock/libsane_mock.c new file mode 100644 index 0000000..35862ab --- /dev/null +++ b/tests/mock/libsane_mock.c @@ -0,0 +1,234 @@ +/* + * Mock libsane backend for the SaneDev_snap params-cache regression test. + * + * Emulates the epsonscan2 backend behavior (see epsonscan2_bug.md): + * - sane_get_parameters() returns SANE_STATUS_INVAL on a second call after + * sane_start (finding 18: the wrapper's get_parameters succeeds, but + * snap()'s redundant call fails without the params-cache fix). + * - sane_read() accepts any request size and serves a deterministic byte + * stream (finding 13: the real backend accepts per-scanline reads). + * + * Loaded via LD_PRELOAD in a dedicated subprocess (see + * tests/test_snap_chunked_read.py) so it only shadows libsane inside the + * test's controlled process. + * + * Scenario knobs (environment variables): + * SNAP_MOCK_DEPTH 1 or 8 (default 8) + * SNAP_MOCK_PARTIAL 1 = truncate the final scanline by 7 bytes + */ +#include + +#include +#include + +#define MOCK_DEVICE "mock:0" +#define MOCK_LINES 2280 +#define MOCK_PPL_8 2280 +#define MOCK_BPL_8 2280 +#define MOCK_PPL_1 1700 +#define MOCK_BPL_1 213 + +static SANE_Int g_depth = 8; +static SANE_Int g_partial = 0; +static long g_offset = 0; +static int g_started = 0; +static int g_get_params_count = 0; + +static void +mock_configure(void) +{ + const char *v; + if((v = getenv("SNAP_MOCK_DEPTH"))) + g_depth = atoi(v); + if((v = getenv("SNAP_MOCK_PARTIAL"))) + g_partial = atoi(v); +} + +/* Deterministic stream: byte k = (k*37 + 11) % 256 */ +static SANE_Byte +mock_byte(long k) +{ + return (SANE_Byte)(((k * 37 + 11) % 256)); +} + +static long +mock_stream_len(void) +{ + long full = MOCK_LINES * (g_depth == 1 ? MOCK_BPL_1 : MOCK_BPL_8); + if(g_partial) + full -= 7; + return full; +} + +SANE_Status +sane_init(SANE_Int *version_code, SANE_Auth_Callback authorize) +{ + (void)authorize; + mock_configure(); + g_offset = 0; + g_started = 0; + g_get_params_count = 0; + if(version_code) + *version_code = SANE_VERSION_CODE(SANE_CURRENT_MAJOR, + SANE_CURRENT_MINOR, 0); + return SANE_STATUS_GOOD; +} + +void +sane_exit(void) +{ +} + +SANE_Status +sane_get_devices(const SANE_Device ***device_list, SANE_Bool local_only) +{ + (void)local_only; + static const SANE_Device device = { + MOCK_DEVICE, "Mock Vendor", "Mock Scanner", "virtual device" + }; + static const SANE_Device *devices[] = { &device, NULL }; + *device_list = devices; + return SANE_STATUS_GOOD; +} + +SANE_Status +sane_open(SANE_String_Const devicename, SANE_Handle *handle) +{ + (void)devicename; + *handle = (SANE_Handle)&g_started; + g_started = 0; + g_get_params_count = 0; + g_offset = 0; + return SANE_STATUS_GOOD; +} + +void +sane_close(SANE_Handle handle) +{ + (void)handle; +} + +void +sane_cancel(SANE_Handle handle) +{ + (void)handle; +} + +const SANE_Option_Descriptor * +sane_get_option_descriptor(SANE_Handle handle, SANE_Int option) +{ + (void)handle; + (void)option; + return NULL; +} + +SANE_Status +sane_control_option(SANE_Handle handle, SANE_Int option, SANE_Action action, + void *value, SANE_Int *info) +{ + (void)handle; + (void)option; + (void)action; + (void)value; + (void)info; + return SANE_STATUS_INVAL; +} + +SANE_Status +sane_get_select_fd(SANE_Handle handle, SANE_Int *fd) +{ + (void)handle; + (void)fd; + return SANE_STATUS_INVAL; +} + +SANE_Status +sane_start(SANE_Handle handle) +{ + (void)handle; + g_started = 1; + g_get_params_count = 0; + return SANE_STATUS_GOOD; +} + +SANE_Status +sane_get_parameters(SANE_Handle handle, SANE_Parameters *params) +{ + (void)handle; + + /* Replicate epsonscan2 behavior: the first sane_get_parameters call after + sane_start succeeds; subsequent calls return INVAL. This is the bug that + finding 18 identified - the python-sane wrapper's get_parameters() call + succeeds, but snap()'s redundant second call fails. */ + if(g_started) + { + ++g_get_params_count; + if(g_get_params_count > 1) + return SANE_STATUS_INVAL; + } + + memset(params, 0, sizeof(*params)); + params->format = SANE_FRAME_GRAY; + params->last_frame = SANE_TRUE; + if(g_depth == 1) + { + params->pixels_per_line = MOCK_PPL_1; + params->lines = MOCK_LINES; + params->depth = 1; + params->bytes_per_line = MOCK_BPL_1; + } + else + { + params->pixels_per_line = MOCK_PPL_8; + params->lines = MOCK_LINES; + params->depth = 8; + params->bytes_per_line = MOCK_BPL_8; + } + return SANE_STATUS_GOOD; +} + +SANE_Status +sane_read(SANE_Handle handle, SANE_Byte *data, SANE_Int max_length, + SANE_Int *length) +{ + (void)handle; + *length = 0; + + if(!g_started) + return SANE_STATUS_INVAL; + + long stream_len = mock_stream_len(); + if(g_offset >= stream_len) + return SANE_STATUS_EOF; + + long avail = stream_len - g_offset; + SANE_Int n = max_length < avail ? max_length : (SANE_Int)avail; + long k = g_offset; + SANE_Int i; + for(i = 0; i < n; ++i, ++k) + data[i] = mock_byte(k); + g_offset += n; + *length = n; + return SANE_STATUS_GOOD; +} + +SANE_String_Const +sane_strstatus(SANE_Status status) +{ + switch(status) + { + case SANE_STATUS_GOOD: return "Success"; + case SANE_STATUS_UNSUPPORTED: return "Operation is not supported"; + case SANE_STATUS_CANCELLED: return "Operation was cancelled"; + case SANE_STATUS_DEVICE_BUSY: return "Device busy"; + case SANE_STATUS_INVAL: return "Invalid argument"; + case SANE_STATUS_EOF: return "End of file reached"; + case SANE_STATUS_JAMMED: return "Document feeder jammed"; + case SANE_STATUS_NO_DOCS: return "Document feeder out of documents"; + case SANE_STATUS_COVER_OPEN: return "Scanner cover is open"; + case SANE_STATUS_IO_ERROR: return "I/O error"; + case SANE_STATUS_NO_MEM: return "Out of memory"; + case SANE_STATUS_ACCESS_DENIED: return "Access denied"; + default: return "Unknown status"; + } +} diff --git a/tests/mock/run_snap_mock.py b/tests/mock/run_snap_mock.py new file mode 100644 index 0000000..7854e4a --- /dev/null +++ b/tests/mock/run_snap_mock.py @@ -0,0 +1,75 @@ +#!/usr/bin/env python3 +"""Run a snap() scan against the LD_PRELOADed mock libsane and verify the +returned image bytes against the deterministic stream the mock serves. + +Invoked as a subprocess by tests/test_snap_params_cache.py with LD_PRELOAD +set to the compiled mock shim. Exits 0 on success, non-zero otherwise. + +Scenario knobs must match those passed to the mock (see libsane_mock.c). +""" +import os +import sys +from pathlib import Path + +sys.path.insert(0, os.environ.get("SNAP_MOCK_REPO_ROOT", + str(Path(__file__).resolve().parents[2]))) + +import sane # noqa: E402 (needs the repo root on sys.path first) + + +def mock_pattern(k): + return (k * 37 + 11) % 256 + + +def main(): + depth = int(os.environ.get("SNAP_MOCK_DEPTH", "8")) + partial = int(os.environ.get("SNAP_MOCK_PARTIAL", "0")) + skip_get_params = os.environ.get("SNAP_MOCK_SKIP_GET_PARAMS", "0") == "1" + + if depth == 1: + ppl, bpl, lines = 1700, 213, 2280 + elif depth == 8: + ppl, bpl, lines = 2280, 2280, 2280 + else: + print("unsupported SNAP_MOCK_DEPTH", depth) + return 2 + + sane.init() + dev = sane.open("mock:0") + dev.start() + + if not skip_get_params: + params = dev.get_parameters() + fmt, last_frame, (ppl2, lines2), depth2, bpl2 = params + assert (fmt, last_frame, ppl2, lines2, depth2, bpl2) == \ + ("gray", 1, ppl, lines, depth, bpl), params + + data, width, height, samples, sample_size = dev.dev.snap() + + stream_len = lines * bpl - (7 if partial else 0) + stream = bytes(mock_pattern(k) for k in range(stream_len)) + + if depth == 1: + expected = bytearray(lines * ppl) + for line in range(lines): + base = line * ppl + sbase = line * bpl + for x in range(ppl): + b = stream[sbase + x // 8] + expected[base + x] = 0 if ((b >> (7 - (x % 8))) & 1) else 255 + expected = bytes(expected) + else: + expected = stream + (bytes(7) if partial else b"") + + assert (width, height, samples, sample_size) == (ppl, lines, 1, 1), \ + (width, height, samples, sample_size) + assert len(data) == len(expected), (len(data), len(expected)) + assert bytes(data) == expected, "image bytes mismatch" + + print("OK depth=%d partial=%d skip_get_params=%d" % (depth, partial, + skip_get_params)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_snap_params_cache.py b/tests/test_snap_params_cache.py new file mode 100644 index 0000000..f58c61d --- /dev/null +++ b/tests/test_snap_params_cache.py @@ -0,0 +1,73 @@ +"""Regression test for the SaneDev_snap params-cache fix (finding 18). + +The epsonscan2 backend returns INVAL on a second sane_get_parameters() call +after sane_start. The wrapper's get_parameters() succeeds; snap()'s redundant +call inside the C extension fails. The fix caches params from +SaneDev_get_parameters and reuses them in SaneDev_snap. + +Runs a real snap() through the C extension against an LD_PRELOADed mock +libsane that replicates this behavior. The mock is loaded via LD_PRELOAD in +a dedicated subprocess so it only shadows libsane inside the test's +controlled process. +""" +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +MOCK_SRC = ROOT / "tests" / "mock" / "libsane_mock.c" +RUNNER = ROOT / "tests" / "mock" / "run_snap_mock.py" +SHIM = ROOT / "build" / "libsane_mock.so" + + +@pytest.fixture(scope="module", autouse=True) +def shim(): + (ROOT / "build").mkdir(exist_ok=True) + subprocess.run( + ["gcc", "-shared", "-fPIC", "-O2", "-o", str(SHIM), str(MOCK_SRC)], + check=True, + ) + if not list(ROOT.glob("_sane.cpython-*.so")): + subprocess.run( + [sys.executable, "setup.py", "build_ext", "--inplace"], + cwd=ROOT, + check=True, + ) + return SHIM + + +def run_scenario(env_extra): + env = os.environ.copy() + env["LD_PRELOAD"] = str(SHIM) + env.update(env_extra) + proc = subprocess.run( + [sys.executable, str(RUNNER)], + env=env, + cwd=ROOT, + capture_output=True, + text=True, + timeout=120, + ) + assert proc.returncode == 0, "scenario failed:\n%s\n%s" % ( + proc.stdout, + proc.stderr, + ) + + +def test_depth8(): + run_scenario({"SNAP_MOCK_DEPTH": "8"}) + + +def test_depth1(): + run_scenario({"SNAP_MOCK_DEPTH": "1"}) + + +def test_snap_without_prior_get_parameters(): + """snap() without a prior get_parameters() should still succeed via the + fallback path in SaneDev_snap (finding 18 fix: cached params avoid a + second sane_get_parameters call, but when the cache is empty snap() + calls sane_get_parameters once as a fallback).""" + run_scenario({"SNAP_MOCK_DEPTH": "8", "SNAP_MOCK_SKIP_GET_PARAMS": "1"})