diff --git a/docs/BUILTINS.md b/docs/BUILTINS.md index d3aa8d3d..1c8bb916 100644 --- a/docs/BUILTINS.md +++ b/docs/BUILTINS.md @@ -567,8 +567,8 @@ axis. Three controls bound it: | Name | Signature | Description | |------|-----------|-------------| -| `http_route` | `http_route of [method, path, handler]` or `[method, path, "code", source]` | Register route handler (literal body or per-request `code` source) | -| `http_route_authed` | `http_route_authed of [method, path, handler]` or `[method, path, "code", source]` | Register authenticated route; auth source published via `shared_set of ["require_auth", ""]` | +| `http_route` | `http_route of [method, path, body]` or `[method, path, "code", source]` | Register a route. `body` is a literal response body, **not** a callback — passing a function raises (#877); use the `code` form for per-request logic | +| `http_route_authed` | `http_route_authed of [method, path, body]` or `[method, path, "code", source]` | Register authenticated route; auth source published via `shared_set of ["require_auth", ""]` | | `http_static` | `http_static of [prefix, directory]` | Serve static files (realpath-confined to `directory`) | | `http_early_bind` | `http_early_bind of null` | Pre-bind socket and start health thread | | `http_serve` | `http_serve of port` | Start blocking HTTP server | diff --git a/docs/STDLIB.md b/docs/STDLIB.md index 6dfa7813..8123792a 100644 --- a/docs/STDLIB.md +++ b/docs/STDLIB.md @@ -874,8 +874,8 @@ Client helpers only allow `http://` and `https://` URLs; rejected URLs return `[ |----------|-----------|-------------| | `http_get` | `http_get of url` | GET request | | `http_post_json` | `http_post_json of [url, data]` | POST JSON | -| `route_get` | `route_get of [path, handler]` | Register GET route | -| `route_post` | `route_post of [path, handler]` | Register POST route | +| `route_get` | `route_get of [path, body]` | Register GET route with a literal body (not a callback — see `http_route`) | +| `route_post` | `route_post of [path, body]` | Register POST route with a literal body (not a callback — see `http_route`) | | `json_response` | `json_response of data` | Build JSON response | | `text_response` | `text_response of string` | Build text response | | `error_response` | `error_response of [code, msg]` | Build error response | diff --git a/lib/http.eigs b/lib/http.eigs index 5ab5bab5..28928301 100644 --- a/lib/http.eigs +++ b/lib/http.eigs @@ -57,21 +57,26 @@ define http_post_json(url, data) as: # Server helpers (require http extension builtins) # ============================================================ -# ---- route_get: register a GET handler ---- -define route_get(path, handler) as: - http_route of ["GET", path, handler] +# #877: the parameter is a literal response BODY, not a callback. These +# wrappers used to name it `handler`, which is what invites passing a +# function — http_route now raises on one rather than serving ``. +# For per-request logic use the code form: http_route of [m, p, "code", src]. -# ---- route_post: register a POST handler ---- -define route_post(path, handler) as: - http_route of ["POST", path, handler] +# ---- route_get: register a GET route with a literal body ---- +define route_get(path, body) as: + http_route of ["GET", path, body] -# ---- route_authed_get: register an authenticated GET handler ---- -define route_authed_get(path, handler) as: - http_route_authed of ["GET", path, handler] +# ---- route_post: register a POST route with a literal body ---- +define route_post(path, body) as: + http_route of ["POST", path, body] -# ---- route_authed_post: register an authenticated POST handler ---- -define route_authed_post(path, handler) as: - http_route_authed of ["POST", path, handler] +# ---- route_authed_get: authenticated GET route with a literal body ---- +define route_authed_get(path, body) as: + http_route_authed of ["GET", path, body] + +# ---- route_authed_post: authenticated POST route with a literal body ---- +define route_authed_post(path, body) as: + http_route_authed of ["POST", path, body] # ---- json_response: build a JSON response string ---- define json_response(data) as: diff --git a/src/ext_http.c b/src/ext_http.c index b1729c97..d88d897b 100644 --- a/src/ext_http.c +++ b/src/ext_http.c @@ -216,11 +216,24 @@ void* health_thread(void *arg) { * HTTP BUILTINS * ================================================================ */ +/* #877: no route slot takes a callback. Every slot — method, path, kind, + * body/source — is stringified into the route table, and VAL_FN/VAL_BUILTIN + * are the only value types with no sensible rendering: they produce the debug + * reprs `` / ``. Dicts, lists, numbers, buffers and + * text-builders all stringify to something a client can use, so this is the + * whole of the class. The trap is foreseeable from the signature alone — + * `handler` means "callback" in every mainstream framework, while here it is + * a literal body — and the failure is silent AND remote-visible: a live + * endpoint answers 200 with `` and nothing is logged. */ +static int route_slot_is_callable(const Value *v) { + return v && (v->type == VAL_FN || v->type == VAL_BUILTIN); +} + Value* builtin_http_route(Value *arg) { /* #356: registration failures must raise — the return value is never * checked, so a silent make_null() means the route just never exists. */ if (!arg || arg->type != VAL_LIST || arg->data.list.count < 3) { - rt_error(EK_TYPE, 0, "http_route requires [method, path, handler...] (3+ elements)"); + rt_error(EK_TYPE, 0, "http_route requires [method, path, body...] (3+ elements)"); return make_null(); } if (g_server.route_count >= MAX_ROUTES) { @@ -228,6 +241,38 @@ Value* builtin_http_route(Value *arg) { return make_null(); } + /* #877: reject callables BEFORE anything is allocated or stored. rt_error + * sets the error flag and returns rather than unwinding, so raising after + * the value_to_string calls below would strand method/path in a route slot + * that route_count never reaches — a leak on the error path. Registration + * time is also the right moment: the error lands on the line the author + * wrote, before the socket is listening, instead of on a client request. */ + for (int i = 0; i < 2; i++) { + if (route_slot_is_callable(arg->data.list.items[i])) { + rt_error(EK_TYPE, 0, "http_route: %s must be a string, not a function", + i == 0 ? "method" : "path"); + return make_null(); + } + } + if (arg->data.list.count >= 4) { + if (route_slot_is_callable(arg->data.list.items[2])) { + rt_error(EK_TYPE, 0, "http_route: kind must be a string, not a function " + "(expected \"code\" or \"static\")"); + return make_null(); + } + if (route_slot_is_callable(arg->data.list.items[3])) { + rt_error(EK_TYPE, 0, "http_route: the code form's source must be a string of " + "EigenScript source, not a function — " + "http_route of [method, path, \"code\", \"return 42\"]"); + return make_null(); + } + } else if (route_slot_is_callable(arg->data.list.items[2])) { + rt_error(EK_TYPE, 0, "http_route: body must be a value, not a function — pass a " + "literal body (\"pong\"), or use the code form: " + "http_route of [method, path, \"code\", \"\"]"); + return make_null(); + } + Route *r = &g_server.routes[g_server.route_count]; char *method_s = value_to_string(arg->data.list.items[0]); char *path_s = value_to_string(arg->data.list.items[1]); diff --git a/tests/test_http_server.sh b/tests/test_http_server.sh index e1db8676..0573f211 100755 --- a/tests/test_http_server.sh +++ b/tests/test_http_server.sh @@ -764,6 +764,75 @@ kill "$SRV3_PID" 2>/dev/null || true wait "$SRV3_PID" 2>/dev/null || true rm -f "$SRV3" "$TAPE3" /tmp/eigs_http_srv3_$$.log +# ---- #877: a callable in any route slot raises at REGISTRATION ------------ +# http_route's third element is a response body, not a callback — but the +# signature names it `handler`, so reaching for it with framework habits is +# foreseeable. It used to stringify: a live endpoint answered 200 with the +# debug repr ``, silently and remote-visibly. VAL_FN/VAL_BUILTIN are +# the only value types with no sensible body rendering, so they are the whole +# of the class; dicts/lists/numbers must still register. +SRV4=$(mktemp /tmp/eigs_http_srv4_XXXXXX.eigs) +cat > "$SRV4" <<'EIGS' +define hello as: + return "hi" +slots is [["GET", "/f", hello], # 3-el body: a function + ["GET", "/b", len], # 3-el body: a builtin + ["GET", "/c", "code", hello], # 4-el code form: source + [hello, "/m", "x"], # method + ["GET", hello, "x"], # path + ["GET", "/k", hello, "src"]] # 4-el kind +for s in slots: + try: + http_route of s + print of "REGISTERED-A-CALLABLE" + catch e: + print of ("raised: " + e["kind"]) +# Every non-callable body still registers. +print of (http_route of ["GET", "/ok", "pong"]) +print of (http_route of ["GET", "/co", "code", "return 42"]) +print of (http_route of ["GET", "/d", {"a": 1}]) +print of (http_route of ["GET", "/n", 42]) +EIGS +OUT4=$("$EIGS" "$SRV4" 2>&1) +RAISED=$(printf '%s\n' "$OUT4" | grep -c '^raised: type_mismatch') +REGD=$(printf '%s\n' "$OUT4" | grep -c '^route registered') +if [ "$RAISED" = "6" ] && [ "$REGD" = "4" ]; then + ok "HS35 every callable route slot raises; every valid body still registers" +else + fail "HS35 callable route slots (#877)" \ + "raised=$RAISED (want 6) registered=$REGD (want 4); output: $OUT4" +fi +if printf '%s\n' "$OUT4" | grep -q 'REGISTERED-A-CALLABLE'; then + fail "HS35 a callable slot registered silently" "output: $OUT4" +else + ok "HS35 no callable slot registers silently" +fi + +# The error must land at registration, before the socket is listening — so an +# uncaught one aborts the program and http_serve is never reached. +PORT4=$(pick_port) +SRV5=$(mktemp /tmp/eigs_http_srv5_XXXXXX.eigs) +cat > "$SRV5" <&1); RC5=$? +if [ "$RC5" = "124" ]; then + fail "HS35 callable body reached http_serve — server ran until killed" "output: $OUT5" +elif [ "$RC5" != "0" ] && ! printf '%s\n' "$OUT5" | grep -q 'UNREACHABLE-SERVE'; then + ok "HS35 an uncaught callable body aborts before http_serve (rc=$RC5)" +else + fail "HS35 callable body did not stop startup" "rc=$RC5 output: $OUT5" +fi +rm -f "$SRV4" "$SRV5" + echo "HTTP_SERVER: $PASS passed, $FAIL failed" if [ "$FAIL" -gt 0 ]; then exit 1; fi exit 0