diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cb8b9b567..5567fb11c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -126,16 +126,16 @@ jobs: uses: actions/cache@v5 with: path: /opt/openssl-3.6 - key: openssl-3.6.0-${{ runner.os }}-${{ runner.arch }} + key: openssl-3.6.2-${{ runner.os }}-${{ runner.arch }} - name: Build OpenSSL 3.6 if: steps.cache-openssl36.outputs.cache-hit != 'true' run: | sudo apt-get -y install build-essential cd /tmp - wget -q https://www.openssl.org/source/openssl-3.6.0.tar.gz - tar -xzf openssl-3.6.0.tar.gz - cd openssl-3.6.0 + wget -q https://www.openssl.org/source/openssl-3.6.2.tar.gz + tar -xzf openssl-3.6.2.tar.gz + cd openssl-3.6.2 ./Configure --prefix=/opt/openssl-3.6 shared no-docs make -j$(nproc) sudo mkdir -p /opt/openssl-3.6 @@ -394,6 +394,14 @@ jobs: ## Tests ## + - name: Build fake_upstream + if: matrix.build == 'unit' || matrix.build == 'python-3.11' || matrix.build == 'python-3.12' + run: | + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y + source "$HOME/.cargo/env" + cargo build --release --manifest-path test/fake_upstream/Cargo.toml + sudo cp test/fake_upstream/target/release/fake_upstream /usr/local/bin/ + # /home/runner will be root only after calling sudo above # Ensure all users and processes can execute - name: Fix permissions diff --git a/.gitignore b/.gitignore index fa7319141..0fe4d3737 100644 --- a/.gitignore +++ b/.gitignore @@ -18,3 +18,8 @@ __pycache__/ /FIX.md /CLAUDE.original.md /.qwen/skills/clang-ast/SKILL.md +/.qwen/.qwen/settings.json +/.qwen/.qwen/settings.json.orig +/.qwen/.qwen/skills/clang-ast/SKILL.md +test/fake_upstream/target/ +test/fake_upstream/Cargo.lock diff --git a/CHANGES b/CHANGES index 8ae835854..9313867fe 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,36 @@ -Changes with FreeUnit 1.35.4 xx xxx 2026 +Changes with FreeUnit 1.35.5 29 May 2026 + + *) Feature: automatically convert chunked request bodies to Content-Length + when forwarding to upstream servers via proxy action. This enables + compatibility with backends that do not support Transfer-Encoding: + chunked (e.g., Gitea, servers requiring Content-Length). Fixes + freeunitorg/freeunit#58, resolves nginx/unit#445 (client chunked), + nginx/unit#1088 (duplicate TE), and nginx/unit#1278 (RFC 9112 epic). + + *) Change: chunked_transform feature is no longer experimental. Chunked + request bodies can be accepted and transparently converted to + Content-Length via configuration: { "settings": { "http": + { "chunked_transform": true } } } + + *) Bugfix: fix TLS library busy-loop on peer-initiated close in SSL_write + when connection is aborted by remote peer; prevents high CPU usage and + ensures proper connection cleanup. + + *) Feature: add unfreeze-sync.sh script for automated migration of issues + from nginx/unit to freeunitorg/freeunit with label mapping, deduplication, + and dry-run preview support. + + *) Change: upgrade contrib njs to 0.9.8. + + *) Bugfix: fix mem-pool retain leak in cert/script-store IPC paths + (router side) and fd/buffer leaks in cert/script/socket/access-log + reply paths and the controller config-store path (main process + side); all reachable when nxt_port_msg_alloc fails inside the + port machinery. + + +Changes with FreeUnit 1.35.4 30 Apr 2026 *) Bugfix: fix router process CPU spin and connection hang under port scanning load; CLOSE-WAIT sockets are now cleaned up properly on diff --git a/TODO.md b/TODO.md index 515b8d826..30791049c 100644 --- a/TODO.md +++ b/TODO.md @@ -135,8 +135,11 @@ Before the OpenSSL 3.6 migration can be considered fully validated: `OBJ_sn2nid` / `OpenSSL_version_num` replacements. - [ ] Run the full CI matrix (`ci.yml`) and confirm the new "Build OpenSSL 3.6" step succeeds on both `amd64` and `arm64` runners. -- [ ] Confirm `clang-ast` workflow passes end-to-end on `debian:testing` - (was broken by `EVP_PKEY_asn1_find_str` / `SSLeay` deprecations). +- [x] `clang-ast` workflow passes on `debian:testing` + system OpenSSL 1.1 + via `./test/run-local-full.sh` (verified on `pre-1.35.5` branch). +- [ ] Confirm `clang-ast` still passes when linked against OpenSSL 3.6 + (previously broken by `EVP_PKEY_asn1_find_str` / `SSLeay` deprecations + — fixes need re-verification on the 3.6 build). - [ ] Smoke-test TLS in a Docker image built from `Dockerfile.minimal` (now `debian:trixie-slim`) — load a certificate via the REST API and make an HTTPS request. @@ -180,3 +183,56 @@ inside a chroot/rootfs-isolated Unit application. 1. Run `ldd $(which php)` with PHP 8.5 and compare against the rootfs fixture contents 2. Check `unit.log` for the full path that caused the segfault (needs core dump or `strace`) 3. Check if `php 8.5 --define open_basedir=...` reproduces outside of Unit + +--- + +## Test Infrastructure + +### Prebuild `fake_upstream` binary via packages.freeunit.org + +`test/fake_upstream/` — Rust HTTP mock used by `test_proxy_chunked.py`. +Currently built from source in Docker (`cargo build --release`), adding ~0.5s per run. + +**Improvement:** +- [ ] Build `fake_upstream` binary and publish to `packages.freeunit.org` +- [ ] Update `run-local.sh` to download prebuilt binary instead of `cargo build` +- [ ] Add SHA-512 checksum validation (like `pkg/contrib/Makefile` does for njs/wasmtime) +- [ ] Fallback to cargo build if download fails + +**Benefits:** +- Faster test image builds +- Reproducible binaries across platforms (AMD64 + ARM64) +- No Rust toolchain required in Docker image + +--- + +### clang-ast Docker build: debian:testing + `clang llvm-dev libclang-dev` + +`test/run-local-full.sh` builds a Docker image for clang-ast analysis. +Fixed: use `clang llvm-dev libclang-dev` (not `clang-21 llvm-21-dev libclang-21-dev`). + +**Current state:** Works on `debian:testing` (clang 21 + llvm 21). + +**Future improvements:** +- [ ] Prebuild `freeunit-test-full:local` image and publish to GHCR +- [ ] Or add packages.freeunit.org binary for clang-ast plugin +- [ ] Cache Docker layers for apt install + clang-ast build + +--- + +## Chunked Encoding (RFC 9112) — Implemented in pre-1.35.5-i58 + +Branch `pre-1.35.5-i58` implements automatic chunked → Content-Length conversion +for proxy request forwarding. Key files: + +- `src/nxt_h1proto.c` — buffer fix (L1149-1171) + CL injection (L2414-2475) +- `test/test_proxy_chunked.py` — 10 tests (all passing) +- `test/fake_upstream/` — Rust HTTP mock with strict CL validation + +**Tests:** 10/10 passed ✅ +**clang-ast:** PASSED ✅ + +**Pending upstream:** +- Consider making the conversion configurable (currently always-on when `r->chunked`) +- Add metrics/counter for chunked → CL conversions +- Consider adding `Transfer-Encoding` removal for HTTP/2 upstream (HTTP/2 doesn't use TE header) diff --git a/src/nxt_cert.c b/src/nxt_cert.c index 600a6c011..e048d0716 100644 --- a/src/nxt_cert.c +++ b/src/nxt_cert.c @@ -1114,7 +1114,6 @@ nxt_cert_store_get(nxt_task_t *task, nxt_str_t *name, nxt_mp_t *mp, goto fail; } - nxt_mp_retain(mp); b->completion_handler = nxt_cert_buf_completion; nxt_buf_cpystr(b, name); @@ -1138,6 +1137,13 @@ nxt_cert_store_get(nxt_task_t *task, nxt_str_t *name, nxt_mp_t *mp, goto fail; } + /* + * Retain only after the buffer has been handed off to the port machinery, + * so that the failure paths above do not leave the pool with a refcount + * that the completion handler can never release. + */ + nxt_mp_retain(mp); + return; fail: @@ -1224,8 +1230,21 @@ nxt_cert_store_get_handler(nxt_task_t *task, nxt_port_recv_msg_t *msg) error: - (void) nxt_port_socket_write(task, port, type, file.fd, - msg->port_msg.stream, 0, NULL); + if (nxt_port_socket_write(task, port, type, file.fd, + msg->port_msg.stream, 0, NULL) + != NXT_OK + && file.fd != -1) + { + /* + * On send failure (e.g. malloc failure inside the port machinery) + * the port layer never takes ownership of the fd, so close it + * here to avoid leaking an open file descriptor in the privileged + * main process. Use nxt_fd_close() rather than nxt_file_close(): + * file.name has already been freed above and the latter would + * dereference it through "%FN" on a close-failure log path. + */ + nxt_fd_close(file.fd); + } } diff --git a/src/nxt_controller.c b/src/nxt_controller.c index a7c6842c7..eec3c9ff8 100644 --- a/src/nxt_controller.c +++ b/src/nxt_controller.c @@ -2446,6 +2446,7 @@ nxt_controller_conf_store(nxt_task_t *task, nxt_conf_value_t *conf) u_char *end; size_t size; nxt_fd_t fd; + nxt_int_t rc; nxt_buf_t *b; nxt_port_t *main_port; nxt_runtime_t *rt; @@ -2479,9 +2480,22 @@ nxt_controller_conf_store(nxt_task_t *task, nxt_conf_value_t *conf) b->mem.free = nxt_cpymem(b->mem.pos, &size, sizeof(size_t)); - (void) nxt_port_socket_write(task, main_port, - NXT_PORT_MSG_CONF_STORE | NXT_PORT_MSG_CLOSE_FD, - fd, 0, -1, b); + rc = nxt_port_socket_write(task, main_port, + NXT_PORT_MSG_CONF_STORE | NXT_PORT_MSG_CLOSE_FD, + fd, 0, -1, b); + + if (nxt_slow_path(rc != NXT_OK)) { + /* + * Port layer did not take ownership of fd or b (e.g. malloc + * failure inside nxt_port_msg_alloc); close the shm fd and + * queue the buffer completion so the engine memory pool is + * not left with an unreclaimed buffer. + */ + nxt_fd_close(fd); + + nxt_work_queue_add(&task->thread->engine->fast_work_queue, + b->completion_handler, task, b, b->parent); + } return; diff --git a/src/nxt_h1proto.c b/src/nxt_h1proto.c index dda3900e3..b3a82a262 100644 --- a/src/nxt_h1proto.c +++ b/src/nxt_h1proto.c @@ -1149,6 +1149,24 @@ nxt_h1p_conn_request_body_read(nxt_task_t *task, void *obj, void *data) if (h1p->chunked_parse.last) { body_rest = 0; + + } else if (h1p->chunked_parse.chunk_size > 0) { + /* Mid-chunk: chunk_parse consumed the entire buffer but did not + * advance b->mem.pos (CHUNK_MIDDLE path in chunk_buffer). + * Reset so nxt_conn_read has space on the next iteration. */ + b->mem.free = b->mem.start; + b->mem.pos = b->mem.start; + + } else { + /* Between chunks: chunk_parse advanced b->mem.pos past all + * framing. Compact any leftover bytes to the front so + * nxt_conn_read appends after them. */ + size = (size_t) (b->mem.free - b->mem.pos); + if (size > 0) { + nxt_memmove(b->mem.start, b->mem.pos, size); + } + b->mem.free = b->mem.start + size; + b->mem.pos = b->mem.start; } } else { @@ -2380,6 +2398,7 @@ nxt_h1p_peer_header_send(nxt_task_t *task, nxt_http_peer_t *peer) nxt_conn_t *c; nxt_http_field_t *field; nxt_http_request_t *r; + nxt_off_t content_length; nxt_debug(task, "h1p peer header send"); @@ -2395,9 +2414,34 @@ nxt_h1p_peer_header_send(nxt_task_t *task, nxt_http_peer_t *peer) + sizeof("Connection: close\r\n") + sizeof("\r\n"); + /* If request body needs Content-Length (e.g., after chunked_transform), + calculate it from the buffered body. Empty chunked body (0\r\n\r\n + only) leaves r->body == NULL — still emit Content-Length: 0 for + backends that require it. */ + content_length = -1; + if (r->chunked) { + if (r->body == NULL) { + content_length = 0; + } else { + nxt_buf_t *b; + + content_length = 0; + + for (b = r->body; b != NULL; b = b->next) { + if (nxt_buf_is_file(b)) { + content_length += b->file_end - b->file_pos; + } else { + content_length += nxt_buf_mem_used_size(&b->mem); + } + } + } + /* Account for Content-Length header size (max off_t length + "Content-Length: \r\n"). */ + size += nxt_length("Content-Length: ") + NXT_OFF_T_LEN + nxt_length("\r\n"); + } + nxt_list_each(field, r->fields) { - if (!field->hopbyhop) { + if (!field->hopbyhop && !field->skip) { size += field->name_length + field->value_length; size += nxt_length(": \r\n"); } @@ -2419,7 +2463,7 @@ nxt_h1p_peer_header_send(nxt_task_t *task, nxt_http_peer_t *peer) nxt_list_each(field, r->fields) { - if (!field->hopbyhop) { + if (!field->hopbyhop && !field->skip) { p = nxt_cpymem(p, field->name, field->name_length); *p++ = ':'; *p++ = ' '; p = nxt_cpymem(p, field->value, field->value_length); @@ -2428,6 +2472,12 @@ nxt_h1p_peer_header_send(nxt_task_t *task, nxt_http_peer_t *peer) } nxt_list_loop; + if (content_length >= 0) { + p = nxt_cpymem(p, "Content-Length: ", nxt_length("Content-Length: ")); + p = nxt_sprintf(p, header->mem.end, "%O", content_length); + *p++ = '\r'; *p++ = '\n'; + } + *p++ = '\r'; *p++ = '\n'; header->mem.free = p; size = p - header->mem.pos; diff --git a/src/nxt_main_process.c b/src/nxt_main_process.c index 25798ea0b..8860e4c8c 100644 --- a/src/nxt_main_process.c +++ b/src/nxt_main_process.c @@ -1170,8 +1170,30 @@ nxt_main_port_socket_handler(nxt_task_t *task, nxt_port_recv_msg_t *msg) type = NXT_PORT_MSG_RPC_ERROR; } - nxt_port_socket_write(task, port, type, ls.socket, msg->port_msg.stream, - 0, out); + if (nxt_port_socket_write(task, port, type, ls.socket, msg->port_msg.stream, + 0, out) + != NXT_OK) + { + /* + * ls.socket is -1 unless nxt_main_listening_socket() succeeded. + * In that case the port layer did not take ownership, so close it + * explicitly. + */ + if (ls.socket != -1) { + nxt_socket_close(task, ls.socket); + } + + /* + * The buffer never reached the port queue, so the port layer will + * not run its completion. Queue the completion to match normal + * port-layer cleanup semantics. + */ + if (out != NULL) { + nxt_work_queue_add(&task->thread->engine->fast_work_queue, + out->completion_handler, task, out, + out->parent); + } + } } @@ -1728,8 +1750,18 @@ nxt_main_port_access_log_handler(nxt_task_t *task, nxt_port_recv_msg_t *msg) msg->port_msg.reply_port); if (nxt_fast_path(port != NULL)) { - (void) nxt_port_socket_write(task, port, type, file.fd, - msg->port_msg.stream, 0, NULL); + if (nxt_port_socket_write(task, port, type, file.fd, + msg->port_msg.stream, 0, NULL) + != NXT_OK + && file.fd != -1) + { + /* + * Port layer never took ownership of the fd (e.g. malloc + * failure inside nxt_port_msg_alloc); close it explicitly to + * avoid leaking the open file in the main process. + */ + nxt_file_close(task, &file); + } } else { nxt_file_close(task, &file); diff --git a/src/nxt_openssl.c b/src/nxt_openssl.c index 265542aae..013ee97c0 100644 --- a/src/nxt_openssl.c +++ b/src/nxt_openssl.c @@ -1592,11 +1592,28 @@ nxt_openssl_conn_test_error(nxt_task_t *task, nxt_conn_t *c, int ret, return NXT_ERROR; } + if (io == NXT_OPENSSL_WRITE) { + /* Cannot write to a closed connection. */ + c->socket.error = sys_err ? sys_err : NXT_ECONNRESET; + return NXT_ERROR; + } + /* A connection was just closed. */ c->socket.closed = 1; return 0; case SSL_ERROR_ZERO_RETURN: + /* + * SSL_write() may return SSL_ERROR_ZERO_RETURN when the peer has + * sent a TLS close_notify. Writing to a half-closed session is + * not possible; treat it as a fatal write error so the connection + * is closed rather than spinning the write loop. + */ + if (io == NXT_OPENSSL_WRITE) { + c->socket.error = NXT_ECONNRESET; + return NXT_ERROR; + } + /* A "close notify" alert. */ return 0; diff --git a/src/nxt_port.h b/src/nxt_port.h index 772fb41ae..df61a407b 100644 --- a/src/nxt_port.h +++ b/src/nxt_port.h @@ -333,6 +333,18 @@ void nxt_port_write_enable(nxt_task_t *task, nxt_port_t *port); void nxt_port_write_close(nxt_port_t *port); void nxt_port_read_enable(nxt_task_t *task, nxt_port_t *port); void nxt_port_read_close(nxt_port_t *port); + +/* + * Ownership contract: + * On NXT_OK, ownership of fd, fd2, and b transfers to the port layer: + * the port layer will close the descriptor(s) and run b's completion + * handler. + * On any other return, ownership remains with the caller, which is + * responsible for closing fd/fd2 if owned and dispatching b's + * completion handler. + * The inline nxt_port_socket_write() wrapper below inherits this + * contract. + */ nxt_int_t nxt_port_socket_write2(nxt_task_t *task, nxt_port_t *port, nxt_uint_t type, nxt_fd_t fd, nxt_fd_t fd2, uint32_t stream, nxt_port_id_t reply_port, nxt_buf_t *b); diff --git a/src/nxt_script.c b/src/nxt_script.c index 05d9561da..486e357cc 100644 --- a/src/nxt_script.c +++ b/src/nxt_script.c @@ -480,7 +480,6 @@ nxt_script_store_get(nxt_task_t *task, nxt_str_t *name, nxt_mp_t *mp, goto fail; } - nxt_mp_retain(mp); b->completion_handler = nxt_script_buf_completion; nxt_buf_cpystr(b, name); @@ -504,6 +503,13 @@ nxt_script_store_get(nxt_task_t *task, nxt_str_t *name, nxt_mp_t *mp, goto fail; } + /* + * Retain only after the buffer has been handed off to the port machinery, + * so that the failure paths above do not leave the pool with a refcount + * that the completion handler can never release. + */ + nxt_mp_retain(mp); + return; fail: @@ -589,8 +595,21 @@ nxt_script_store_get_handler(nxt_task_t *task, nxt_port_recv_msg_t *msg) error: - (void) nxt_port_socket_write(task, port, type, file.fd, - msg->port_msg.stream, 0, NULL); + if (nxt_port_socket_write(task, port, type, file.fd, + msg->port_msg.stream, 0, NULL) + != NXT_OK + && file.fd != -1) + { + /* + * On send failure the port layer never takes ownership of the fd, + * so close it here to avoid leaking an open file descriptor in the + * privileged main process. Use nxt_fd_close() rather than + * nxt_file_close(): file.name has already been freed above and + * the latter would dereference it through "%FN" on a close-failure + * log path. + */ + nxt_fd_close(file.fd); + } } diff --git a/test/README.md b/test/README.md index b78f8e71a..cfd0d57ce 100644 --- a/test/README.md +++ b/test/README.md @@ -1,5 +1,22 @@ # FreeUnit Test Suite +## CRITICAL: Docker-only + +**ALL build and test commands MUST run inside Docker.** Never run +`./configure`, `make`, `pytest-3`, `python3`, or any language runtime +directly on the host — host drift hides bugs that surface in CI. + +Use `./test/run-local.sh` (preferred). For one-shot commands, override +the fixed `ENTRYPOINT` (which is `bash -c "...build...exec pytest-3 $@"`) +and mount at `/unit` (the image `WORKDIR`): + +```bash +docker run --rm --entrypoint bash -v "$(pwd):/unit" -w /unit \ + freeunit-test:local -c '' +``` + +See project-root `CLAUDE.md` for the full allowed/forbidden list. + ## Running Tests Tests require **root privileges** because Unit creates Unix domain sockets, @@ -83,6 +100,9 @@ sudo pytest-3 --print-log --restart test/ # 7. Save logs after execution sudo pytest-3 --print-log --save-log test/ + +# 8. Run clang-ast AST analysis (C-code quality check) +./test/run-local.sh --clang-ast ``` ## Test Structure diff --git a/test/conftest.py b/test/conftest.py index 3a313c29c..e5ab92161 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -471,6 +471,7 @@ def _check_processes(): router_pid = _fds_info['router']['pid'] controller_pid = _fds_info['controller']['pid'] main_pid = unit_instance['pid'] + main_pid_re = re.compile(r'\b' + re.escape(main_pid) + r'\b') for _ in range(600): out = ( @@ -480,7 +481,7 @@ def _check_processes(): .decode() .splitlines() ) - out = [l for l in out if main_pid in l] + out = [l for l in out if main_pid_re.search(l)] if len(out) <= 3: break diff --git a/test/fake_upstream/Cargo.lock b/test/fake_upstream/Cargo.lock new file mode 100644 index 000000000..b7e93d06a --- /dev/null +++ b/test/fake_upstream/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "fake_upstream" +version = "0.1.0" diff --git a/test/fake_upstream/Cargo.toml b/test/fake_upstream/Cargo.toml new file mode 100644 index 000000000..e7c04114d --- /dev/null +++ b/test/fake_upstream/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "fake_upstream" +version = "0.1.0" +edition = "2021" + +[[bin]] +name = "fake_upstream" +path = "src/main.rs" diff --git a/test/fake_upstream/src/main.rs b/test/fake_upstream/src/main.rs new file mode 100644 index 000000000..a894f24f3 --- /dev/null +++ b/test/fake_upstream/src/main.rs @@ -0,0 +1,279 @@ +/// fake_upstream — live HTTP/1.x mock upstream for FreeUnit proxy tests. +/// +/// Usage: +/// fake_upstream --port --mode [--requests ] +/// +/// Modes: +/// requires-cl 411 if Transfer-Encoding: chunked without Content-Length +/// no-te 400 if Transfer-Encoding present; 411 if no Content-Length +/// strict 400 if Transfer-Encoding present; 411 if no CL; +/// 400 if body length != CL value; else 200 + echo +/// echo 200 + echo body (no header enforcement) +/// +/// --requests N exit after handling N connections (default: run forever) + +use std::env; +use std::io::{BufRead, BufReader, Read, Write}; +use std::net::{TcpListener, TcpStream}; +use std::process; +use std::str; + +// --------------------------------------------------------------------------- + +#[derive(Clone, Copy, PartialEq, Debug)] +enum Mode { + RequiresCl, + NoTe, + Strict, + Echo, +} + +// --------------------------------------------------------------------------- + +struct Request { + method: String, + headers: Vec<(String, String)>, + body: Vec, +} + +impl Request { + fn header_val(&self, name: &str) -> Option<&str> { + self.headers + .iter() + .find(|(k, _)| k.eq_ignore_ascii_case(name)) + .map(|(_, v)| v.as_str()) + } + + fn has_header(&self, name: &str) -> bool { + self.header_val(name).is_some() + } + + fn has_te_chunked(&self) -> bool { + self.header_val("Transfer-Encoding") + .map(|v| v.to_ascii_lowercase().contains("chunked")) + .unwrap_or(false) + } + + fn content_length(&self) -> Option { + self.header_val("Content-Length")?.trim().parse().ok() + } +} + +// --------------------------------------------------------------------------- + +fn read_request(stream: &TcpStream) -> std::io::Result { + let mut reader = BufReader::new(stream); + + // Request line + let mut req_line = String::new(); + reader.read_line(&mut req_line)?; + let method = req_line.split_whitespace().next().unwrap_or("GET").to_string(); + + // Headers + let mut headers: Vec<(String, String)> = Vec::new(); + loop { + let mut line = String::new(); + reader.read_line(&mut line)?; + let trimmed = line.trim_end_matches(['\r', '\n']); + if trimmed.is_empty() { + break; + } + if let Some(pos) = trimmed.find(':') { + let name = trimmed[..pos].trim().to_string(); + let value = trimmed[pos + 1..].trim().to_string(); + headers.push((name, value)); + } + } + + // Body — detect encoding + let te_chunked = headers + .iter() + .any(|(k, v)| { + k.eq_ignore_ascii_case("transfer-encoding") + && v.to_ascii_lowercase().contains("chunked") + }); + + let body = if te_chunked { + read_chunked_body(&mut reader)? + } else { + let cl: usize = headers + .iter() + .find(|(k, _)| k.eq_ignore_ascii_case("content-length")) + .and_then(|(_, v)| v.trim().parse().ok()) + .unwrap_or(0); + let mut buf = vec![0u8; cl]; + if cl > 0 { + reader.read_exact(&mut buf)?; + } + buf + }; + + Ok(Request { method, headers, body }) +} + +fn read_chunked_body(reader: &mut BufReader<&TcpStream>) -> std::io::Result> { + let mut body = Vec::new(); + loop { + let mut size_line = String::new(); + reader.read_line(&mut size_line)?; + // Strip chunk extensions (RFC 9112: chunk-size [";" chunk-ext]). + let size_field = size_line + .trim_end_matches(['\r', '\n']) + .split(';') + .next() + .unwrap_or("") + .trim(); + let chunk_size = usize::from_str_radix(size_field, 16).unwrap_or(0); + if chunk_size == 0 { + // consume trailing CRLF after terminal chunk + let mut crlf = String::new(); + reader.read_line(&mut crlf)?; + break; + } + let mut chunk = vec![0u8; chunk_size]; + reader.read_exact(&mut chunk)?; + body.extend_from_slice(&chunk); + // consume trailing CRLF after chunk data + let mut crlf = String::new(); + reader.read_line(&mut crlf)?; + } + Ok(body) +} + +// --------------------------------------------------------------------------- + +fn respond(stream: &mut TcpStream, status: u16, reason: &str, body: &[u8]) { + let head = format!( + "HTTP/1.1 {} {}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n", + status, + reason, + body.len() + ); + let _ = stream.write_all(head.as_bytes()); + let _ = stream.write_all(body); +} + +fn handle(mut stream: TcpStream, mode: Mode) { + let req = match read_request(&stream) { + Ok(r) => r, + Err(_) => { + respond(&mut stream, 400, "Bad Request", b"failed to parse request"); + return; + } + }; + + match mode { + Mode::Echo => { + respond(&mut stream, 200, "OK", &req.body); + } + + Mode::RequiresCl => { + if req.has_te_chunked() && !req.has_header("Content-Length") { + respond(&mut stream, 411, "Length Required", b"Content-Length required"); + return; + } + respond(&mut stream, 200, "OK", &req.body); + } + + Mode::NoTe => { + if req.has_header("Transfer-Encoding") { + respond(&mut stream, 400, "Bad Request", b"Transfer-Encoding not allowed"); + return; + } + if !req.has_header("Content-Length") && req.method != "GET" && req.method != "HEAD" { + respond(&mut stream, 411, "Length Required", b"Content-Length required"); + return; + } + respond(&mut stream, 200, "OK", &req.body); + } + + Mode::Strict => { + if req.has_header("Transfer-Encoding") { + respond(&mut stream, 400, "Bad Request", b"Transfer-Encoding not allowed"); + return; + } + match req.content_length() { + None => { + respond(&mut stream, 411, "Length Required", b"Content-Length required"); + return; + } + Some(cl) if cl != req.body.len() => { + let msg = format!( + "Content-Length mismatch: header={} body={}", + cl, + req.body.len() + ); + respond(&mut stream, 400, "Bad Request", msg.as_bytes()); + return; + } + _ => {} + } + respond(&mut stream, 200, "OK", &req.body); + } + } +} + +// --------------------------------------------------------------------------- + +fn usage() -> ! { + eprintln!( + "Usage: fake_upstream --port --mode [--requests ]" + ); + process::exit(1); +} + +fn main() { + let args: Vec = env::args().collect(); + + let mut port: Option = None; + let mut mode: Option = None; + let mut max_requests: Option = None; + + let mut i = 1; + while i < args.len() { + match args[i].as_str() { + "--port" => { + i += 1; + port = args.get(i).and_then(|v| v.parse().ok()); + } + "--mode" => { + i += 1; + mode = args.get(i).and_then(|v| match v.as_str() { + "requires-cl" => Some(Mode::RequiresCl), + "no-te" => Some(Mode::NoTe), + "strict" => Some(Mode::Strict), + "echo" => Some(Mode::Echo), + _ => None, + }); + } + "--requests" => { + i += 1; + max_requests = args.get(i).and_then(|v| v.parse().ok()); + } + _ => {} + } + i += 1; + } + + let port = port.unwrap_or_else(|| usage()); + let mode = mode.unwrap_or_else(|| usage()); + + let listener = TcpListener::bind(("127.0.0.1", port)).unwrap_or_else(|e| { + eprintln!("bind {}:{} — {}", "127.0.0.1", port, e); + process::exit(1); + }); + + let mut count = 0usize; + for stream in listener.incoming() { + match stream { + Ok(s) => { + handle(s, mode); + count += 1; + if max_requests.map_or(false, |n| count >= n) { + break; + } + } + Err(_) => break, + } + } +} diff --git a/test/run-local-full.sh b/test/run-local-full.sh new file mode 100755 index 000000000..b6b80a21e --- /dev/null +++ b/test/run-local-full.sh @@ -0,0 +1,150 @@ +#!/usr/bin/env bash +# run-local-full.sh — pre-commit / PR static analysis via clang-ast +# +# Runs the FreeUnit C build under the freeunitorg/clang-ast LLVM plugin +# inside Docker. Catches API-misuse / lifetime / allocator violations the +# normal compile does not. Intended to be run BEFORE every commit and PR. +# +# Scope: C core only — configure is `--openssl --debug`. Module-specific +# C code (njs, otel, brotli, zlib, zstd) is NOT analyzed by this run. +# For those, use ./test/run-local.sh which builds the full feature set. +# +# Usage: +# ./test/run-local-full.sh # build + clang-ast check +# ./test/run-local-full.sh -n # dry-run (print, don't execute) +# +# To force image rebuild: docker rmi freeunit-test-full:local + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_DIR="$(cd "${SCRIPT_DIR}/.." && pwd)" +IMAGE_NAME="freeunit-test-full:local" +DRY_RUN=false +TMP_DIR="" + +log() { echo "[$(date '+%H:%M:%S')] $*"; } +info() { log "INFO $*"; } +err() { log "ERROR $*" >&2; } + +case "${1:-}" in + -n) DRY_RUN=true ;; + -h|--help) + sed -n '/^# Usage:/,/^[^#]/{ /^#/s/^# \{0,3\}//p }' "$0" + exit 0 + ;; + "") ;; + *) err "Unknown option: $1"; exit 1 ;; +esac + +if ! command -v docker &>/dev/null; then + err "docker not found in PATH"; exit 1 +fi + +# --------------------------------------------------------------------------- +# Tmp copy — isolate source from live tree +# --------------------------------------------------------------------------- +prepare_tmp() { + TMP_DIR="$(mktemp -d /tmp/freeunit-test-full.XXXXXX)" + if $DRY_RUN; then + info "Dry-run: would copy project → $TMP_DIR (skipping)" + return 0 + fi + info "Copying project → $TMP_DIR" + rsync -a --exclude='.git' --exclude='/build' "${PROJECT_DIR}/" "${TMP_DIR}/" +} + +cleanup_tmp() { + if [[ -n "$TMP_DIR" && -d "$TMP_DIR" ]]; then + info "Tmp dir left at $TMP_DIR (cleanup disabled)" + # rm -rf "$TMP_DIR" + fi +} +trap cleanup_tmp EXIT + +# --------------------------------------------------------------------------- +# Build the clang-ast image if missing +# --------------------------------------------------------------------------- +build_image() { + if docker image inspect "$IMAGE_NAME" &>/dev/null; then + info "Using cached image: $IMAGE_NAME" + return 0 + fi + + info "Building clang-ast image: $IMAGE_NAME (one-time, slow)" + + local DOCKERFILE + DOCKERFILE="$(mktemp /tmp/Dockerfile.full.XXXXXX)" + + cat > "$DOCKERFILE" <<'EOF' +FROM debian:testing + +LABEL org.opencontainers.image.title="FreeUnit (full / clang-ast)" +LABEL org.opencontainers.image.vendor="FreeUnit Community " + +ENV DEBIAN_FRONTEND=noninteractive + +RUN set -ex \ + && apt-get update \ + && apt-get install --no-install-recommends --no-install-suggests -y \ + ca-certificates git build-essential libssl-dev libpcre2-dev \ + zlib1g-dev libzstd-dev libbrotli-dev curl pkg-config pkgconf \ + clang llvm-dev libclang-dev \ + && git clone https://github.com/freeunitorg/clang-ast.git -b unit /clang-ast \ + && make -C /clang-ast + +WORKDIR /unit + +# Source mounted via -v at runtime. Configure + build WITH clang-ast plugin +# to catch API/lifetime/allocator violations. Plugin emits diagnostics during +# compile; non-zero exit = failure. +ENTRYPOINT ["bash", "-c", "\ + set -ex && \ + NCPU=$(getconf _NPROCESSORS_ONLN) && \ + rm -rf build && \ + CC=clang ./configure \ + --cc-opt='-Xclang -load -Xclang /clang-ast/ngx-ast.so \ + -Xclang -add-plugin -Xclang ngx-ast \ + -Wno-default-const-init-field-unsafe' \ + --openssl --debug && \ + make -j $NCPU && \ + echo '=== clang-ast check PASSED ===' \ +"] +EOF + + if $DRY_RUN; then + info "Dry-run: would build with:" + cat "$DOCKERFILE" + rm -f "$DOCKERFILE" + return 0 + fi + + docker build --file "$DOCKERFILE" --tag "$IMAGE_NAME" "${PROJECT_DIR}" + rm -f "$DOCKERFILE" +} + +# --------------------------------------------------------------------------- +# Run clang-ast +# --------------------------------------------------------------------------- +run_check() { + info "Running clang-ast AST analysis" + + if $DRY_RUN; then + info "Dry-run: would execute:" + echo " docker run --rm -v ${TMP_DIR}:/unit -w /unit ${IMAGE_NAME}" + return 0 + fi + + docker run --rm \ + --name "freeunit-test-full" \ + -v "${TMP_DIR}:/unit" \ + -w /unit \ + "${IMAGE_NAME}" +} + +# --------------------------------------------------------------------------- +# Main +# --------------------------------------------------------------------------- +prepare_tmp +build_image +run_check diff --git a/test/run-local.sh b/test/run-local.sh index 876e88cb3..bdc8dfa65 100755 --- a/test/run-local.sh +++ b/test/run-local.sh @@ -23,6 +23,8 @@ # ./run-local.sh -t test_a.py -t test_b.py # multiple test files # ./run-local.sh unit python php # multiple modules # +# For pre-commit / PR static analysis (clang-ast): ./test/run-local-full.sh +# # To force rebuild: docker rmi freeunit-test:local set -euo pipefail @@ -70,26 +72,29 @@ usage() { MODULES=() TEST_ARGS=() -while getopts ":m:t:nh" opt; do - case $opt in - m) MODULES+=("$OPTARG") ;; - t) TEST_ARGS+=("$OPTARG") ;; - n) DRY_RUN=true ;; - h) usage ;; - :) err "Option -$OPTARG requires an argument."; exit 1 ;; - \?) err "Unknown option: -$OPTARG"; exit 1 ;; +while [[ $# -gt 0 ]]; do + case "$1" in + -m) MODULES+=("$2"); shift 2 ;; + -t) TEST_ARGS+=("$2"); shift 2 ;; + -n) DRY_RUN=true; shift ;; + -h) usage ;; + -*) err "Unknown option: $1"; exit 1 ;; + *) MODULES+=("$1"); shift ;; esac done -shift $((OPTIND - 1)) - -# Positional args are module names -MODULES+=("$@") # Defaults: no -t and no modules → run full unit test suite if [[ ${#TEST_ARGS[@]} -eq 0 ]] && [[ ${#MODULES[@]} -eq 0 ]]; then MODULES=("unit") fi +# Normalize: prepend test/ when -t is bare test_*.py or test_*.py::nodeid +for i in "${!TEST_ARGS[@]}"; do + if [[ "${TEST_ARGS[$i]}" == test_* && "${TEST_ARGS[$i]}" != test/* ]]; then + TEST_ARGS[$i]="test/${TEST_ARGS[$i]}" + fi +done + # Expand modules → test path patterns (only when -t not given) if [[ ${#TEST_ARGS[@]} -eq 0 ]] && [[ ${#MODULES[@]} -gt 0 ]]; then for m in "${MODULES[@]}"; do @@ -227,6 +232,8 @@ ENTRYPOINT ["bash", "-c", "\ printf 'NXT_INCS += -I%s/pkg/contrib/njs/src -I%s/pkg/contrib/njs/build\\n' \ $(pwd) $(pwd) >> build/Makefile && \ make python3 && \ + cargo build --release --manifest-path test/fake_upstream/Cargo.toml && \ + cp test/fake_upstream/target/release/fake_upstream /usr/local/bin/fake_upstream && \ exec pytest-3 --print-log $@ \ ", "bash"] diff --git a/test/test_proxy_chunked.py b/test/test_proxy_chunked.py index 23476cd93..9729f2dea 100644 --- a/test/test_proxy_chunked.py +++ b/test/test_proxy_chunked.py @@ -1,6 +1,8 @@ +import os import re import select import socket +import subprocess import time import pytest @@ -22,6 +24,11 @@ def setup_method_fixture(): assert 'success' in client.conf( { + "settings": { + "http": { + "chunked_transform": True + } + }, "listeners": { "*:8080": {"pass": "routes"}, }, @@ -227,3 +234,179 @@ def check_invalid(body): ) >= 1048576 ) + + +def _recvall(sock): + buff_size = 4096 * 4096 + data = b'' + while True: + rlist = select.select([sock], [], [], 0.1) + if not rlist[0]: + break + part = sock.recv(buff_size) + data += part + if not part: + break + return data + + +def _serve_loop(server_port, handler): + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) + sock.bind(('127.0.0.1', server_port)) + sock.listen(10) + + while True: + connection, _ = sock.accept() + data = _recvall(connection).decode() + connection.sendall(handler(data).encode()) + connection.close() + + +def _echo_body(data): + m = re.search('\x0d\x0a\x0d\x0a(.*)', data, re.M | re.S) + return m.group(1) if m is not None else '' + + +def _get_free_port(): + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(('127.0.0.1', 0)) + return s.getsockname()[1] + + +def _run_proxy_server(handler, port): + run_process(_serve_loop, port, handler) + waitforsocket(port) + + +def _handler_sends_chunked(_data): + body_content = 'Hello, World!' + return ( + "HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n" + f"{len(body_content):x}\r\n{body_content}\r\n0\r\n\r\n" + ) + + +def _configure_proxy(port, label): + assert 'success' in client.conf( + { + "settings": {"http": {"chunked_transform": True}}, + "listeners": {"*:8080": {"pass": "routes"}}, + "routes": [{"action": {"proxy": f'http://127.0.0.1:{port}'}}], + } + ), label + + +# --------------------------------------------------------------------------- +# fake_upstream — live Rust HTTP mock (test/fake_upstream/) +# --------------------------------------------------------------------------- + +FAKE_UPSTREAM_BIN = '/usr/local/bin/fake_upstream' + +# Graceful skip when the Rust mock binary is not built (e.g. local runs +# without a Rust toolchain). CI installs it via the "Build fake_upstream" +# step in ci.yml. +_skipif_no_fake_upstream = pytest.mark.skipif( + not os.path.exists(FAKE_UPSTREAM_BIN), + reason=f'{FAKE_UPSTREAM_BIN} not installed (build via test/fake_upstream)', +) + + +def _chunked_encode(data: bytes) -> bytes: + """Encode bytes as a single-chunk chunked body.""" + return f'{len(data):x}\r\n'.encode() + data + b'\r\n0\r\n\r\n' + + +def _run_fake_upstream(mode: str, port: int): + proc = subprocess.Popen( + [FAKE_UPSTREAM_BIN, '--port', str(port), '--mode', mode], + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + ) + waitforsocket(port) + return proc + + +def _fake_get(body: bytes, **kwargs): + """Send HTTP/1.1 GET with Transfer-Encoding: chunked to FreeUnit proxy.""" + return client.get( + headers={'Transfer-Encoding': 'chunked', 'Connection': 'close'}, + body=_chunked_encode(body).decode('latin-1'), + **kwargs, + ) + + +@_skipif_no_fake_upstream +def test_proxy_chunked_to_content_length(): + """Chunked request → Content-Length for backends that require it. (Issue #1278)""" + port = _get_free_port() + proc = _run_fake_upstream('requires-cl', port) + try: + _configure_proxy(port, 'requires-cl backend') + body = b'test' + resp = _fake_get(body) + assert resp['status'] == 200, 'backend must accept after chunked→CL conversion' + assert resp['body'].encode('latin-1') == body, 'dechunked body must match exactly' + finally: + proc.terminate() + proc.wait() + + +@_skipif_no_fake_upstream +def test_proxy_chunked_no_duplicate_transfer_encoding(): + """Proxied request must not carry Transfer-Encoding after conversion. (Issue #1088)""" + port = _get_free_port() + proc = _run_fake_upstream('no-te', port) + try: + _configure_proxy(port, 'no-te backend') + body = b'data' + resp = _fake_get(body) + assert resp['status'] == 200, 'backend must not see Transfer-Encoding header' + assert resp['body'].encode('latin-1') == body, 'dechunked body must match exactly' + finally: + proc.terminate() + proc.wait() + + +@_skipif_no_fake_upstream +def test_proxy_chunked_cl_matches_body(): + """Content-Length value must equal actual dechunked body size. (Issue #445, #58)""" + port = _get_free_port() + proc = _run_fake_upstream('strict', port) + try: + _configure_proxy(port, 'strict backend') + body = b'hello world' + resp = _fake_get(body) + assert resp['status'] == 200, 'strict backend: CL must equal dechunked body length' + assert resp['body'].encode('latin-1') == body, 'dechunked body must match exactly' + finally: + proc.terminate() + proc.wait() + + +def test_app_response_chunked_not_duplicated(): + """App's Transfer-Encoding: chunked response is not duplicated. (Issue #1088)""" + port = _get_free_port() + _run_proxy_server(_handler_sends_chunked, port) + _configure_proxy(port, 'chunked-response backend') + + resp = get_http10() + assert resp['status'] == 200, 'response should be OK' + assert resp['body'] == 'Hello, World!', 'body must match exactly' + + +@_skipif_no_fake_upstream +def test_chunked_large_body(): + """Large chunked request (64 KB > body_buffer_size) → correct Content-Length. (Issue #445)""" + port = _get_free_port() + proc = _run_fake_upstream('strict', port) + try: + _configure_proxy(port, 'strict backend for large body') + body = b'0123456789abcdef' * 4096 # 64 KB — forces file buffer path + resp = _fake_get(body, read_buffer_size=65536 * 4) + assert resp['status'] == 200, 'large chunked request must return 200' + assert resp['body'].encode('latin-1') == body, 'dechunked 64 KB body must match exactly' + finally: + proc.terminate() + proc.wait() diff --git a/test/test_tls.py b/test/test_tls.py index ffbcee401..fbbc18454 100644 --- a/test/test_tls.py +++ b/test/test_tls.py @@ -559,6 +559,65 @@ def test_tls_no_close_notify(): sock.close() +def test_tls_write_abrupt_close(): + """Regression: SSL_write busy-loop when client closes mid-response. + + If the router spins, the final get_ssl() will time out and fail. + Covers both SSL_ERROR_SYSCALL(errno=0) and SSL_ERROR_ZERO_RETURN on + the write path (issue #28). + """ + client.load('body_generate') + + client.certificate() + + add_tls(application='body_generate') + + # SSL_write fails with errno=0 → nxt_socket_error_level(0) → NXT_LOG_ALERT. + # These alerts are expected; suppress them so teardown does not fail. + # Match only the syscall/zero-return signatures this test provokes, + # so unrelated SSL_write regressions are not silently masked. + option.skip_alerts += [ + r'SSL_write\([^)]+\) failed \(0: Success\)', + r'SSL_write\([^)]+\) failed \(\d+: Connection reset by peer\)', + r'SSL_write\([^)]+\) failed \(\d+: Broken pipe\)', + ] + + # Body must exceed the kernel send buffer so the server is still + # writing when the client tears the connection down. 16 MB beats + # autotuned SO_SNDBUF on common Linux configurations. + body_size = 16 * 1024 * 1024 + + headers = { + 'Host': 'localhost', + 'Connection': 'close', + 'X-Length': str(body_size), + } + + # Case 1: abrupt TCP close without TLS close_notify. + # Triggers SSL_ERROR_SYSCALL(errno=0 or ECONNRESET) on server write. + sock = client.get_ssl(headers=headers, no_recv=True) + sock.recv(256) + sock.close() + + time.sleep(0.2) + + # Case 2: TLS close_notify while server is still writing. + # Triggers SSL_ERROR_ZERO_RETURN on server write. + sock = client.get_ssl(headers=headers, no_recv=True) + sock.recv(256) + try: + plain = sock.unwrap() + plain.close() + except OSError: + sock.close() + + time.sleep(0.2) + + # Router must still be responsive — not stuck in a busy-loop. + assert client.get_ssl(read_timeout=5).get('status') == 200, \ + 'router hung after aborted TLS write (issue #28)' + + @pytest.mark.skip('not yet') def test_tls_keepalive_certificate_remove(): client.load('empty') diff --git a/version b/version index bd00317ef..77748cf21 100644 --- a/version +++ b/version @@ -1,5 +1,5 @@ # Copyright (C) FreeUnit Community -NXT_VERSION=1.35.4 -NXT_VERNUM=13504 +NXT_VERSION=1.35.5 +NXT_VERNUM=13505