NVDA backend initialize() falsely succeeds when NVDA is not running, shadowing other screen readers (regression in v0.16.2)

[aaronr7734]

Summary

Since v0.16.2 (commit 83f73a8), NvdaBackend::initialize() returns success even when NVDA is not running. Because the NVDA backend is registered at the highest Windows priority (103), BackendRegistry::acquire_best() selects this non-functional backend ahead of backends that are actually running (e.g. JAWS at 100), and subsequent speak()/output() calls fail silently with BackendNotAvailable.

Affected versions

v0.16.2, v0.16.3, v0.16.4. Last good version: v0.16.1.

Root cause

In source/backends/nvda.cpp, initialize():

if (server_supports_interface(controller_handle, nvdaController_NvdaController_v1_0_c_ifspec) &&
    nvdaController_testIfRunning(controller_handle) != ERROR_SUCCESS)
  return std::unexpected(BackendError::BackendNotAvailable);
return {};

When NVDA is not running there is no RPC server at the NvdaCtlr.* ncalrpc endpoint, so server_supports_interface() returns false (its RpcMgmtInqIfIds call fails). The && short-circuits, nvdaController_testIfRunning() is never evaluated, and the function falls through to return {};, reporting the backend as available.

Prior to 83f73a8 the check was simply if (nvdaController_testIfRunning(...) != ERROR_SUCCESS) return ...BackendNotAvailable;, which correctly failed when NVDA was not running.

Impact

NvdaBackend is registered at priority 103, the highest of the Windows backends. acquire_best() returns the first backend (in descending priority) whose initialize() succeeds, so on any Windows machine where NVDA is not running it returns the NVDA backend and never reaches the running screen reader. speak()/output()/braille() then fail with BackendNotAvailable (they still call testIfRunning), leaving callers with a backend that reports a valid name and SUPPORTS_SPEAK but produces no output. Observed in the field with a JAWS user: acquire_best() reported NVDA, no speech was produced, and rolling back to v0.16.1 restored JAWS.

Suggested fix

The testIfRunning liveness check should not be gated behind a short-circuiting &&. Either require both conditions:

if (!server_supports_interface(controller_handle, nvdaController_NvdaController_v1_0_c_ifspec) ||
    nvdaController_testIfRunning(controller_handle) != ERROR_SUCCESS)
  return std::unexpected(BackendError::BackendNotAvailable);

...or simply restore the pre-83f73a8 check (if (nvdaController_testIfRunning(...) != ERROR_SUCCESS) return ...;), which is unconditional and known-good.

I'm happy to open a PR, though I don't have a JAWS setup to verify the screen-reader-selection path — let me know if you'd like one.

Reproduction

1. On Windows, ensure NVDA is not running (another screen reader such as JAWS may be running).
2. Call prism_registry_acquire_best().
3. The returned backend is NVDA, and prism_backend_output() returns BackendNotAvailable / produces no speech.

Environment: Prism v0.16.2–v0.16.4, Windows x64, dynamic release build.

[ethindp]

@aaronr7734 Your suggested fix is not a fix, in that || is a short-circuiting disjunctive and && is a short-circuiting conjunctive. Restoring the old behavior would indeed solve this problem but would also make it possible to impersonate the controller client. The new checks are to prove that we are indeed actually communicating with the controller client. I am about to push a commit which will close this issue, but testing would be appreciated.

[Neurrone]

I have a report of this happening even with V0.16.5, NVDA was being used even when it wasn't running and the user was using ZDSR. Downgrading to 0.16.1 fixed it.

[ethindp]

All right, I will try some tinkering and see if I can figure out what's causing this. I thought I had fixed it in 0.1 6.5 but apparently not.

[ethindp]

@aaronr7734 @Neurrone I just pushed what I believe is a correct fix (the suggested one had the condition inverted, and I mistakenly moved one check within the other). I would appreciate any feedback before I actually consider this solved given that the last time I considered it "solved" was obviously premature.