fix(FR-3058): send empty JSON body for non-GET requests with null body by agatha197 · Pull Request #7651 · lablup/backend.ai-webui

agatha197 · 2026-05-29T03:31:55Z

Summary

When the request body is null/undefined, Client.newSignedRequest() previously sent an empty string '' as the HTTP body. Some backend REST endpoints now require at least a valid empty JSON body ({}) for methods that carry a body (e.g. POST /vfolders/{name}/leave), and reject '' with a 400.

This centralizes the fix in newSignedRequest() so every call site that passes a null body is handled at once:

Send '{}' for non-GET/HEAD methods when no body is provided.
Keep '' for GET/HEAD (query-based requests are unchanged).
Keep authBody as '' so HMAC signatures are unaffected — on API v4+ the body is already excluded from the signature, and on legacy (v<4) backends keeping '' preserves the existing signature.

Affected call sites (previously passing `null`)

VFolder.leave_invited() — POST /vfolders/{name}/leave (confirmed broken without {})
Client.restart() — PATCH /kernel/{id}
Client.refreshSSHKeypair() — PATCH /auth/ssh-keypair
Client.destroy() — DELETE /kernel/{id}
Client.check_login() — POST /server/login-check (SESSION mode)

Endpoints that already pass an explicit object or {} are unaffected. GET requests using query strings are intentionally left as-is.

Related: FR-2978 (#7609) fixed leave_invited() individually; this PR generalizes the same fix and #7609 is stacked on top.

Test plan

Leave a shared (invited) vfolder — request succeeds (no 400).
Restart a session — PATCH /kernel/{id} succeeds.
Refresh SSH keypair — PATCH /auth/ssh-keypair succeeds.
Verify GET requests still send no body (no regression in query-based APIs).

agatha197 · 2026-05-29T03:32:13Z

fix(FR-3058): send empty JSON body for non-GET requests with null body #7651 👈 (View in Graphite)
main

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

flow:merge-queue - adds this PR to the back of the merge queue
flow:hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has required the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

github-actions · 2026-05-29T03:33:32Z

Coverage Report for root-coverage

Status	Category	Percentage	Covered / Total
🔵	Lines	4.77%	28 / 586
🔵	Statements	5.14%	32 / 622
🔵	Functions	7.89%	6 / 76
🔵	Branches	4.98%	18 / 361

File Coverage

No changed files found.

Generated in workflow #1342 for commit 04ccea6 by the Vitest Coverage Report Action

Copilot

Pull request overview

Adjusts the Node/Electron-side Backend.AI client request builder to send a valid empty JSON body ({}) when call sites pass null/undefined for non-GET/HEAD requests, addressing manager endpoints that reject an empty string body with 400.

Changes:

Update Client.newSignedRequest() to send '{}' instead of '' when body is null/undefined for non-GET/HEAD methods.
Preserve '' behavior for GET/HEAD requests.

nowgnuesLee

Please resolve the copilot's reviews.

When body is null/undefined, newSignedRequest() was sending an empty string '' as the request body. Some backends now require at least a valid empty JSON body '{}' for POST/PATCH/DELETE requests. Send '{}' for non-GET/HEAD methods while keeping authBody as '' to preserve HMAC signatures for legacy (v<4) backends.

agatha197 · 2026-06-04T01:44:32Z

@nowgnuesLee Addressed both of Copilot's review comments in the latest commit:

HMAC v<4 body mismatch — authBody now mirrors requestBody instead of being forced to '', so the legacy-backend signature (which hashes the body) matches the actual '{}' body sent. v4+ is unchanged (it already excludes the body from the signature).
Browser client not patched — mirrored the same null-body fix in packages/backend.ai-client/src/client.ts, the client the React WebUI actually uses, so the centralized fix now covers WebUI REST calls too.

Both threads are resolved and scripts/verify.sh passes. PTAL 🙏

nowgnuesLee

If we modify this part, it seems like it could cause side effects across the entire project. Is this the intended area to change? And add a number of jira issue on title and body.

agatha197 · 2026-06-07T16:00:46Z

@nowgnuesLee on the side-effect concern: the change is scoped to the null/undefined-body branch of newSignedRequest() only. Call sites that pass an explicit object or {} are unaffected, and GET/HEAD requests with a null body still send '' (no change). The only behavioral delta is: non-GET/HEAD requests that previously sent an empty-string body now send {}. Those call sites are the ones enumerated in the description (VFolder.leave_invited, Client.restart, refreshSSHKeypair, destroy, check_login) — all non-GET methods that carry a body and were already being rejected by newer managers when sent ''. authBody mirrors requestBody ({} for non-GET/HEAD, '' otherwise) so HMAC signing stays consistent on both v<4 and v4+. So the surface is intentional and bounded; it does not touch GET-based query APIs.

On the Jira number: this generalized fix needs its own issue key (it's broader than FR-2978, which #7609 covers specifically). I'll update the title to fix(FR-XXXX): … and prepend Resolves #<issue> (FR-XXXX) to the body as soon as the issue is assigned — I didn't want to invent a key. Could you point me at (or create) the Jira issue for this one?

agatha197 · 2026-06-08T02:03:30Z

Added the Jira issue: FR-3058 ("Send empty JSON body for non-GET requests with null body in newSignedRequest"), linked as relates to FR-2978. Updated the PR title to fix(FR-3058): … and added the Jira reference to the body. The scope is bounded as explained above — re-requesting review. Thanks!

nowgnuesLee

LGTM

yomybaby · 2026-06-08T10:20:54Z

+      // Send '{}' for methods that carry a body; keep '' for GET/HEAD.
+      // authBody mirrors requestBody so the v<4 signature (which hashes the
+      // body) matches what is actually sent; v4+ excludes the body anyway.
+      requestBody = method !== 'GET' && method !== 'HEAD' ? '{}' : '';


Please check the backend changes related to this part again. The fact that {} must be sent unconditionally is strange.

agatha197 · 2026-06-08T11:09:17Z

Closing in favor of the targeted fix in #7609.

Why: This PR made newSignedRequest() send {} as the body for every non-GET/HEAD request with a null body. That over-applies: only POST /folders/{name}/leave (BodyParam[LeaveVFolderReq]) actually rejects a null body. Worse, it risks a regression on legacy backends: the manager's legacy @check_api_params only falls back to dict(request.query) when the request body is empty (api/utils.py). A blanket {} body makes request.can_read_body true, so check_api_params reads the (empty) body instead of the query string — silently dropping query-string params such as forced and owner_access_key on DELETE /kernel/{id} (destroy) and PATCH /kernel/{id} (restart). A forced terminate would degrade to a graceful one; admin-on-behalf-of scoping would break.

#7609 fixes the real bug in a targeted way (explicit {} in leave_invited only, in both clients) and has been rebased directly onto main (no longer stacked on this PR).

Separately discovered (follow-ups, not addressed here): POST /session/{id}/shutdown-service and POST /session/{id}/download were migrated to BodyParam with required body fields (service_name, files) but the webui still sends those as query string with a null body — these need their own query→body fix, which a blanket {} would not solve.

…400 (#7609) **Jira**: [FR-2978](https://lablup.atlassian.net/browse/FR-2978) > Targets `main` directly. The previously-stacked generalized fix (#7651, which made `newSignedRequest` send `{}` for *every* null-body non-GET request) was dropped: on legacy backends `@check_api_params` only falls back to `dict(request.query)` when the body is empty, so a blanket `{}` body would suppress that fallback and silently drop query-string params on `destroy`/`restart` (e.g. `forced`, `owner_access_key`). This PR keeps the fix targeted to `leave_invited`, the one endpoint whose handler (`BodyParam[LeaveVFolderReq]`) actually rejects a null body. It also extends the client signature with the optional `sharedUserUuid` parameter (admin-on-behalf-of). ## Summary Fixes a 400 error when a user tries to leave a shared folder via the **Shared Folder Permission** modal (both V1 `SharedFolderPermissionInfoModal` and V2 `SharedFolderPermissionInfoModalV2`). ## Root cause The Backend.AI manager's `POST /folders/{name}/leave` REST route parses its body via `BodyParam[LeaveVFolderReq]`. All fields on `LeaveVFolderReq` are optional, but the body itself must still be valid JSON. The frontend client's `VFolder.leave_invited` was passing `null` as the body. `newSignedRequest` serializes `null` to an empty string `''` and sends it with `Content-Type: application/json`. An empty string is not valid JSON, so the manager rejects the request with 400 before the handler runs. Both `SharedFolderPermissionInfoModal.tsx` (V1) and `SharedFolderPermissionInfoModalV2.tsx` (V2) call the same `leave_invited` client method, so the bug reproduces in both flows. ## Change The fix is applied in **both** copies of the API client to keep them consistent: - `packages/backend.ai-client/src/resources/vfolder.ts` — **load-bearing for the React UI**. `react/src/global-stores.ts` registers `globalThis.BackendAIClient = ai.backend.Client` from this workspace package. This is what runs in the browser. - `src/lib/backend.ai-client-node.ts` — legacy ESM client; only consumed by `src/wsproxy/` (Electron desktop's websocket proxy). Kept in sync to avoid a divergent regression if the Electron side ever calls the leave flow. - `react/src/hooks/index.tsx` — extends the `BackendAIClient.vfolder.leave_invited` TypeScript signature with the new optional `sharedUserUuid` parameter (matches `LeaveVFolderReq.shared_user_uuid` for future admin-on-behalf-of flows). Both methods now send `{}` as the JSON body by default. When `sharedUserUuid` is provided, they send `{ shared_user_uuid: sharedUserUuid }`. No existing call site needs to change for the immediate fix — both modals continue calling `leave_invited(folderId)` with a single argument. ## Test plan - [ ] As a regular user with an accepted folder invitation, open the data page → click on the shared folder → permission modal → leave button → confirm. Expect a success toast and the folder to disappear from the list (no 400). - [ ] Repeat for both V1 modal (regular page) and V2 modal (project admin data page). - [ ] CI: lint / format / relay drift / TypeScript pass. ## Verification `bash scripts/verify.sh` was run locally. Relay, Lint, Format pass. The TypeScript step reports pre-existing environmental errors (duplicate `@types/react` between `react/` and `packages/backend.ai-ui/`) that affect files unrelated to this change — CI's clean install is authoritative. [FR-2978]: https://lablup.atlassian.net/browse/FR-2978?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

github-actions Bot assigned agatha197 May 29, 2026

github-actions Bot added area:lib Library and SDK related issue. size:XS ~10 LoC labels May 29, 2026

This was referenced May 29, 2026

fix(FR-2978): send empty JSON body in vfolder leave_invited to avoid 400 #7609

Merged

test(FR-2983): add E2E for leaving a shared (invited) vfolder #7612

Merged

agatha197 marked this pull request as ready for review May 29, 2026 04:39

Copilot AI review requested due to automatic review settings May 29, 2026 04:39

Copilot started reviewing on behalf of agatha197 May 29, 2026 04:39 View session

agatha197 requested review from ironAiken2, nowgnuesLee and yomybaby and removed request for Copilot May 29, 2026 04:40

agatha197 force-pushed the 05-29-fix_send_empty_json_body_for_non-get_requests_with_null_body branch from ed46ce5 to 4e8dcc7 Compare June 2, 2026 06:48

Copilot AI review requested due to automatic review settings June 2, 2026 06:48

Copilot started reviewing on behalf of agatha197 June 2, 2026 06:49 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

Comment thread src/lib/backend.ai-client-node.ts Outdated

Comment thread src/lib/backend.ai-client-node.ts

nowgnuesLee requested changes Jun 3, 2026

View reviewed changes

agatha197 force-pushed the 05-29-fix_send_empty_json_body_for_non-get_requests_with_null_body branch from 4e8dcc7 to 04ccea6 Compare June 4, 2026 01:42

github-actions Bot added size:S 10~30 LoC and removed size:XS ~10 LoC labels Jun 4, 2026

agatha197 requested a review from nowgnuesLee June 4, 2026 01:44

nowgnuesLee requested changes Jun 7, 2026

View reviewed changes

agatha197 changed the title ~~fix: send empty JSON body for non-GET requests with null body~~ fix(FR-3058): send empty JSON body for non-GET requests with null body Jun 8, 2026

agatha197 requested a review from nowgnuesLee June 8, 2026 02:03

nowgnuesLee approved these changes Jun 8, 2026

View reviewed changes

yomybaby requested changes Jun 8, 2026

View reviewed changes

agatha197 closed this Jun 8, 2026

This was referenced Jun 8, 2026

fix(session): send shutdown-service and download params in request body for migrated REST handlers #7777

Open

fix(FR-3070): send shutdown-service & download params in request body #7778

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(FR-3058): send empty JSON body for non-GET requests with null body#7651

fix(FR-3058): send empty JSON body for non-GET requests with null body#7651
agatha197 wants to merge 1 commit into
mainfrom
05-29-fix_send_empty_json_body_for_non-get_requests_with_null_body

agatha197 commented May 29, 2026 •

edited by atlassian Bot

Loading

Uh oh!

agatha197 commented May 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 29, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

nowgnuesLee left a comment

Uh oh!

agatha197 commented Jun 4, 2026

Uh oh!

nowgnuesLee left a comment

Uh oh!

agatha197 commented Jun 7, 2026

Uh oh!

agatha197 commented Jun 8, 2026

Uh oh!

nowgnuesLee left a comment

Uh oh!

yomybaby Jun 8, 2026

Uh oh!

agatha197 commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

agatha197 commented May 29, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Affected call sites (previously passing null)

Test plan

Uh oh!

agatha197 commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

github-actions Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for root-coverage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

nowgnuesLee left a comment

Choose a reason for hiding this comment

Uh oh!

agatha197 commented Jun 4, 2026

Uh oh!

nowgnuesLee left a comment

Choose a reason for hiding this comment

Uh oh!

agatha197 commented Jun 7, 2026

Uh oh!

agatha197 commented Jun 8, 2026

Uh oh!

nowgnuesLee left a comment

Choose a reason for hiding this comment

Uh oh!

yomybaby Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

agatha197 commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

agatha197 commented May 29, 2026 •

edited by atlassian Bot

Loading

Affected call sites (previously passing `null`)

agatha197 commented May 29, 2026 •

edited

Loading

github-actions Bot commented May 29, 2026 •

edited

Loading