try fixing 400 error when no error
All checks were successful
Build and Deploy MITM Webserver / build (push) Successful in 9s
All checks were successful
Build and Deploy MITM Webserver / build (push) Successful in 9s
This commit is contained in:
@@ -512,8 +512,9 @@ def create_rule_json(req: CreateRuleRequest):
|
|||||||
Create a rule from JSON (expr required).
|
Create a rule from JSON (expr required).
|
||||||
- Attempts to render expr -> textual fragment and execute: `add rule <family> <table> <chain> <fragment>`
|
- Attempts to render expr -> textual fragment and execute: `add rule <family> <table> <chain> <fragment>`
|
||||||
- If rendering fails: 400 instructing the client to use POST /firewall/raw
|
- 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
|
- Returns ExecResult on success (201) or on error (400) with stdout/stderr in body.
|
||||||
but still includes the ExecResult body so the client can inspect stderr/stdout.
|
- 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:
|
try:
|
||||||
family = req.family
|
family = req.family
|
||||||
@@ -521,14 +522,10 @@ def create_rule_json(req: CreateRuleRequest):
|
|||||||
chain = req.chain
|
chain = req.chain
|
||||||
|
|
||||||
if req.expr is None:
|
if req.expr is None:
|
||||||
logger.debug("create_rule_json: missing expr in request")
|
|
||||||
raise NftError("field 'expr' is required for JSON rule creation")
|
raise NftError("field 'expr' is required for JSON rule creation")
|
||||||
|
|
||||||
# attempt to render expr -> textual fragment
|
|
||||||
rendered = expr_to_text(req.expr)
|
rendered = expr_to_text(req.expr)
|
||||||
if rendered is None:
|
if rendered is None:
|
||||||
# cannot render deterministically
|
|
||||||
logger.debug("create_rule_json: expr_to_text returned None; advise raw endpoint")
|
|
||||||
raise NftError(
|
raise NftError(
|
||||||
"cannot render provided 'expr' to textual nft syntax. "
|
"cannot render provided 'expr' to textual nft syntax. "
|
||||||
"Please use POST /firewall/raw to execute the textual nft command."
|
"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)
|
logger.info("create_rule_json executing command: %s", cmd)
|
||||||
|
|
||||||
res = mgr.cmd(cmd)
|
res = mgr.cmd(cmd)
|
||||||
# res is dict {"rc": rc, "stdout": out, "stderr": err}
|
# res expected {"rc": rc, "stdout": out, "stderr": err}
|
||||||
rc = int(res.get("rc", -1) or -1)
|
raw_rc = res.get("rc")
|
||||||
stdout = res.get("stdout")
|
stdout = res.get("stdout") or ""
|
||||||
stderr = res.get("stderr")
|
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)
|
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 or None, stderr=stderr or None)
|
||||||
exec_res = ExecResult(rc=rc, stdout=stdout, stderr=stderr)
|
|
||||||
|
|
||||||
# If nft returned non-zero code, surface it as 400 but include the ExecResult in the response body
|
# If rc == 0 — success
|
||||||
if rc != 0:
|
if rc == 0:
|
||||||
# include command and stderr in the HTTP error detail for client UX
|
return exec_res
|
||||||
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)
|
|
||||||
|
|
||||||
# rc == 0 -> success
|
# Handle the annoying case: wrapper returned invalid rc (<0) or non-zero,
|
||||||
return exec_res
|
# 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:
|
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)
|
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))
|
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:
|
except Exception as e:
|
||||||
logger.exception("create_rule_json internal error")
|
logger.exception("create_rule_json internal error")
|
||||||
raise HTTPException(status_code=500, detail=str(e))
|
raise HTTPException(status_code=500, detail=str(e))
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
@router.delete("/rules/{handle}", status_code=status.HTTP_204_NO_CONTENT, summary="Delete rule by handle")
|
@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"):
|
def delete_rule(handle: int, family: str = "inet", table: str = "filter", chain: str = "input"):
|
||||||
"""
|
"""
|
||||||
|
|||||||
Reference in New Issue
Block a user