http module swallows io.ReadAll error on response body #169

New issue

Closed

opened 2026-09-14 18:45:21 +00:00 by advisor-bot · 0 comments

advisor-bot commented

2026-09-14 18:45:21 +00:00

Owner

Where

internal/atis/http.go, in httpReq (around lines 148-160):

responseTable.RawSetString("status_code", lua.LNumber(resp.StatusCode))
responseTable.RawSetString("status", lua.LString(resp.Status))

body, err := io.ReadAll(resp.Body)
if isErr {
    rt.Push(responseTable)
    rt.Push(lua.LString(err.Error()))
    return 2
}
defer func() { _ = resp.Body.Close() }()

Reason

isErr is a local variable that was already used and settled earlier in the function, during header validation (header.ForEach(...)), and is false at this point in the code (otherwise the function would already have returned). The real error from io.ReadAll(resp.Body) is bound to err but is never actually checked — the code checks the stale isErr flag instead.

As a result, if reading the response body fails (connection drop mid-read, timeout, etc.), the function silently returns success (nil error) with a partial or empty body, and the calling Lua script has no way to know the read failed.

Suggested fix

Check the actual error from the read:

body, err := io.ReadAll(resp.Body)
if err != nil {
    rt.Push(responseTable)
    rt.Push(lua.LString(err.Error()))
    return 2
}
defer func() { _ = resp.Body.Close() }()

(Note: resp.Body.Close() is currently deferred after the read — fine functionally, but worth moving before the read/right after checking resp is non-nil, for clarity/consistency with the rest of the codebase.)

## Where `internal/atis/http.go`, in `httpReq` (around lines 148-160): ```go responseTable.RawSetString("status_code", lua.LNumber(resp.StatusCode)) responseTable.RawSetString("status", lua.LString(resp.Status)) body, err := io.ReadAll(resp.Body) if isErr { rt.Push(responseTable) rt.Push(lua.LString(err.Error())) return 2 } defer func() { _ = resp.Body.Close() }() ``` ## Reason `isErr` is a local variable that was already used and settled earlier in the function, during header validation (`header.ForEach(...)`), and is `false` at this point in the code (otherwise the function would already have returned). The real error from `io.ReadAll(resp.Body)` is bound to `err` but is never actually checked — the code checks the stale `isErr` flag instead. As a result, if reading the response body fails (connection drop mid-read, timeout, etc.), the function silently returns success (`nil` error) with a partial or empty `body`, and the calling Lua script has no way to know the read failed. ## Suggested fix Check the actual error from the read: ```go body, err := io.ReadAll(resp.Body) if err != nil { rt.Push(responseTable) rt.Push(lua.LString(err.Error())) return 2 } defer func() { _ = resp.Body.Close() }() ``` (Note: `resp.Body.Close()` is currently deferred *after* the read — fine functionally, but worth moving before the read/right after checking `resp` is non-nil, for clarity/consistency with the rest of the codebase.)