From 0389dd77aaefe7e4dfeeebeb6f1bf26c62619eda Mon Sep 17 00:00:00 2001 From: malmert Date: Wed, 11 Feb 2026 21:41:14 +0100 Subject: [PATCH] try fixing 400 error when no error --- backend/src/api/nft_manager.py | 81 ++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 33 deletions(-) diff --git a/backend/src/api/nft_manager.py b/backend/src/api/nft_manager.py index e98012f..920a5ae 100644 --- a/backend/src/api/nft_manager.py +++ b/backend/src/api/nft_manager.py @@ -512,8 +512,9 @@ def create_rule_json(req: CreateRuleRequest): Create a rule from JSON (expr required). - Attempts to render expr -> textual fragment and execute: `add rule ` - If rendering fails: 400 instructing the client to use POST /firewall/raw - - Returns ExecResult always (rc/stdout/stderr). On success returns 201; on failure returns 400 - but still includes the ExecResult body so the client can inspect stderr/stdout. + - Returns ExecResult on success (201) or on error (400) with stdout/stderr in body. + - If nft wrapper returns an invalid rc but the command produced no stderr, we double-check the chain + to see if the new rule is present; if present we treat as success. """ try: family = req.family @@ -521,14 +522,10 @@ def create_rule_json(req: CreateRuleRequest): chain = req.chain if req.expr is None: - logger.debug("create_rule_json: missing expr in request") raise NftError("field 'expr' is required for JSON rule creation") - # attempt to render expr -> textual fragment rendered = expr_to_text(req.expr) if rendered is None: - # cannot render deterministically - logger.debug("create_rule_json: expr_to_text returned None; advise raw endpoint") raise NftError( "cannot render provided 'expr' to textual nft syntax. " "Please use POST /firewall/raw to execute the textual nft command." @@ -539,47 +536,65 @@ def create_rule_json(req: CreateRuleRequest): logger.info("create_rule_json executing command: %s", cmd) res = mgr.cmd(cmd) - # res is dict {"rc": rc, "stdout": out, "stderr": err} - rc = int(res.get("rc", -1) or -1) - stdout = res.get("stdout") - stderr = res.get("stderr") + # res expected {"rc": rc, "stdout": out, "stderr": err} + raw_rc = res.get("rc") + stdout = res.get("stdout") or "" + stderr = res.get("stderr") or "" + + # Coerce rc to int safely; if not int-like, set -1 to indicate unknown. + try: + rc = int(raw_rc) + except Exception: + rc = -1 - # Log the outputs for debugging logger.info("nft cmd rc=%s stdout=%r stderr=%r cmd=%s", rc, stdout, stderr, cmd) - # Build ExecResult to return in any case (so client gets stdout/stderr) - exec_res = ExecResult(rc=rc, stdout=stdout, stderr=stderr) + exec_res = ExecResult(rc=rc, stdout=stdout or None, stderr=stderr or None) - # If nft returned non-zero code, surface it as 400 but include the ExecResult in the response body - if rc != 0: - # include command and stderr in the HTTP error detail for client UX - detail = f"nft command failed rc={rc}. stderr: {stderr!r}. cmd: {cmd}" - logger.warning("create_rule_json failed: %s", detail) - # Raise HTTPException with ExecResult in the response content: - # FastAPI doesn't let us attach the ExecResult as body when raising, so return explicit response. - # Use HTTPException to set status 400 but include the exec_res in the body by returning it explicitly below. - raise NftError(detail) + # If rc == 0 — success + if rc == 0: + return exec_res - # rc == 0 -> success - return exec_res + # Handle the annoying case: wrapper returned invalid rc (<0) or non-zero, + # but stderr is empty. The command may have succeeded nevertheless. + if (rc < 0 or rc != 0) and stderr.strip() == "": + logger.debug("create_rule_json: rc indicates failure but stderr empty; verifying rule presence") + + # Attempt to verify the rule exists by listing the chain and searching for a textual match. + # We use list_chain_text because it returns textual rule lines we can search for the preview text. + try: + chain_text = mgr.list_chain_text(family, table, chain) or "" + # Simple presence check: the textual fragment we attempted to add should be present + # as a substring in the chain listing (e.g. "ip protocol icmp drop"). + if expr_text and expr_text in chain_text: + logger.info("create_rule_json: detected rule in chain after add; treating as success") + # return success ExecResult with rc=0 to indicate success to client + return ExecResult(rc=0, stdout=stdout or None, stderr=stderr or None) + else: + logger.debug("create_rule_json: rule not found in chain text; chain_text=%r", chain_text) + except Exception as e_chain: + logger.warning("create_rule_json: failed to list chain for verification: %s", e_chain) + + # If we reach here -> treat as error: return 400 with exec_res in body. + # FastAPI cannot both raise HTTPException and include ExecResult as body easily, so raise HTTPException + # with detail that includes stderr and the executed cmd. + detail = f"nft command failed rc={rc}. stderr: {stderr!r}. cmd: {cmd}" + logger.warning("create_rule_json failed: %s", detail) + # Return an HTTPException with the detail (frontend can still inspect error.response.data if ExecResult was included) + raise HTTPException(status_code=400, detail=detail) except NftError as e: - # For NftError we want to return 400 with the ExecResult if available, otherwise simple message. logger.warning("create_rule_json NftError: %s", e) - # If we hit this because of a failed command, try to return ExecResult body with status 400. - # Build a minimal ExecResult with rc=-1 if not available. - # Note: FastAPI won't serialize ExecResult if we raise HTTPException with detail only, - # so return an HTTPException with the message and let client rely on message. However we want body. - # The simplest robust approach is to return an HTTP response manually here. - - # Attempt to find last command outputs from mgr? Not safe. Use message from exception. raise HTTPException(status_code=400, detail=str(e)) - + except HTTPException: + # re-raise HTTPException so we don't wrap it again + raise except Exception as e: logger.exception("create_rule_json internal error") raise HTTPException(status_code=500, detail=str(e)) + @router.delete("/rules/{handle}", status_code=status.HTTP_204_NO_CONTENT, summary="Delete rule by handle") def delete_rule(handle: int, family: str = "inet", table: str = "filter", chain: str = "input"): """