The GitHub Actions job "Release Auditing" on texera.git/backport/7852-report-a-malformed-pve-websocket-handsha-v1.2 has failed. Run started by GitHub user github-actions[bot] (triggered by github-actions[bot]).
Head commit for run: f5bd7189ae727821701f6942f53d2e9e651ab5ef / Xinyuan Lin <[email protected]> fix(amber): report a malformed PVE websocket handshake (#7852) ### What changes were proposed in this PR? `PveWebsocketResource.onOpen` read the handshake parameters in its prologue, outside the `Future` that owns both the `catch` arm that turns a failure into an `[ERR]` line and the pump that writes to the socket. A handshake missing `cuid`, carrying an empty value list for it, or carrying a non-numeric one threw out of `onOpen` itself, ahead of both — so the endpoint's own error contract could not see it: ``` Before: ?pveName=env&action=install -> NumberFormatException out of onOpen -> socket closes, nothing sent After: ?pveName=env&action=install -> [ERR] Missing required parameter: cuid -> __DONE__ ``` The client side makes that silent close worse than a lost message: `computing-unit-selection.component.ts` clears `isInstalling`/`isLocked` only when `onmessage` sees `__DONE__`, and there is no `onclose` handler, so the pip modal stayed locked on "installing" with an empty log until the user navigated away. The three reads move inside the `try`, so a malformed handshake now travels the same path as any other failure — an `[ERR]` line followed by the sentinel. A `requiredParam` helper rejects an absent key, an empty value list, and a blank value alike; the last one is reachable from the wire, since `?pveName=` arrives as `[""]` and used to resolve to a venv directory named `" "` rather than failing. | Handshake | Before | After | | --- | --- | --- | | no `cuid` | NPE out of `onOpen`, socket closes empty | `[ERR] Missing required parameter: cuid` then `__DONE__` | | `cuid` present, value list empty | IndexOutOfBoundsException, same | same as above | | `cuid=abc` | NumberFormatException, same | `[ERR] Invalid cuid: abc` then `__DONE__` | | `pveName=` (blank) | resolved a venv path with a whitespace name | `[ERR] Missing required parameter: pveName` then `__DONE__` | Well-formed handshakes are untouched: the parsed `cuid`/`pveName` reach `PveManager` exactly as before. ### Any related issues, documentation, discussions? Follows #7847, which added `PveWebsocketResourceSpec` and deliberately left this path unpinned so that fixing it would not have to fight a test that had cemented it. ### How was this PR tested? Four cases added to `PveWebsocketResourceSpec` — `cuid` absent, `cuid` with an empty value list, a non-numeric `cuid`, and a blank `pveName` — each asserting the client receives both the `[ERR]` line and the sentinel, and that the pump stops rather than parking in `queue.take()`. ```bash sbt "WorkflowExecutionService/testOnly org.apache.texera.web.resource.pythonvirtualenvironment.PveWebsocketResourceSpec" ``` `Tests: succeeded 7, failed 0`. Written before the fix: all four failed against the old source with the escaping exception itself (`NullPointerException`, `IndexOutOfBoundsException`, `NumberFormatException`) rather than an assertion mismatch, which is the defect stated as a test. Hoisting the reads back out of the `Future` fails all four again. Mutation testing on the new code — 4 mutants, 4 killed: dropping the `values == null` arm, dropping the `isEmpty` arm, hardcoding `cuid` into the message instead of interpolating the parameter name, and swallowing a bad `cuid` as `0`. `PveResourceSpec`, the other suite in the package, is unaffected — locally it reports 6 pre-existing failures that all come from the Windows interpreter path (`Scripts\python.exe`), unrelated to the endpoint. ```bash sbt scalafmtCheckAll "scalafixAll --check" ``` Clean. ### Was this PR authored or co-authored using generative AI tooling? (backported from commit c9a0ddf7ae18f9fd9ecc75732c0b5e93f0203794) Generated-by: Claude Code (Opus 5) Report URL: https://github.com/apache/texera/actions/runs/32618857465 With regards, GitHub Actions via GitBox
