mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(docker): give project-document uploads their own nginx location so they are not capped at 1 MB (#5934)
* fix(docker): give project-document uploads their own nginx location
POST /api/projects/{id}/documents is a multipart upload that Gateway
accepts up to uploads.max_file_size (50 MiB by default), but no nginx
location matched it. It fell through to the /api/ catch-all, which has no
client_max_body_size, so nginx's 1m default answered a 2 MB PDF with a
bare 413 before Gateway saw the request. Reproduced on nginx:alpine with
the shipped config; the same body to /api/threads/{id}/uploads passed.
Add a `~ ^/api/projects/[^/]+/documents` location to the Docker, make dev
and Helm configs with the thread-uploads settings (100M, streamed body)
and the 600s read timeout the catch-all already granted these routes. The
catch-all keeps nginx's defaults. The shared location-block helpers move
to tests/support/nginx_conf.py so the new test and the existing body-size
test use one implementation.
* docs(changelog): reference #5934 in the project-documents nginx entries
This commit is contained in:
parent
19618fc30b
commit
a253faefdf
12
CHANGELOG.md
12
CHANGELOG.md
@ -323,6 +323,17 @@ This release closes that milestone with **181 merged pull requests**.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **docker:** Project-document uploads larger than 1 MB no longer fail with a
|
||||
bare nginx `413` through the unified entry point. `POST
|
||||
/api/projects/{id}/documents` is a multipart upload that Gateway accepts up
|
||||
to `uploads.max_file_size` (50 MiB by default), but no nginx location
|
||||
matched it, so it fell through to the `/api/` catch-all and nginx's default
|
||||
`client_max_body_size 1m` rejected the request before Gateway saw it — a
|
||||
2 MB PDF was refused while the same file uploaded fine into a thread. All
|
||||
three maintained configs (Docker, `make dev`, Helm) now give
|
||||
`/api/projects/{id}/documents` its own location with the thread-uploads
|
||||
settings (100M ceiling, streamed request body) and the read timeout the
|
||||
catch-all already granted; the catch-all itself keeps nginx's defaults. ([#5934])
|
||||
- **docker:** The production stack (`make up` / `scripts/deploy.sh`) now starts
|
||||
on hosts with IPv6 disabled. `docker/nginx/nginx.conf` also listens on
|
||||
`[::]:2026`; on a kernel booted with `ipv6.disable=1` that listen makes nginx
|
||||
@ -6156,4 +6167,5 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#5893]: https://github.com/bytedance/deer-flow/pull/5893
|
||||
[#5900]: https://github.com/bytedance/deer-flow/pull/5900
|
||||
[#5928]: https://github.com/bytedance/deer-flow/pull/5928
|
||||
[#5934]: https://github.com/bytedance/deer-flow/pull/5934
|
||||
|
||||
|
||||
@ -328,6 +328,13 @@
|
||||
|
||||
### 修复
|
||||
|
||||
- **Docker:** 通过统一入口上传超过 1 MB 的项目文档不再被 nginx 直接以 `413` 拒绝。
|
||||
`POST /api/projects/{id}/documents` 是 multipart 上传,Gateway 接受至多
|
||||
`uploads.max_file_size`(默认 50 MiB)的文件,但没有任何 nginx location 匹配它,于是请求
|
||||
落到 `/api/` 兜底块,被 nginx 默认的 `client_max_body_size 1m` 在到达 Gateway 之前拒绝——
|
||||
一个 2 MB 的 PDF 会被拒,而同一个文件上传到会话里却没问题。三份维护中的配置(Docker、
|
||||
`make dev`、Helm)现在都为 `/api/projects/{id}/documents` 单独设置 location,沿用会话上传的
|
||||
设置(100M 上限、请求体流式转发)以及兜底块原本给予的读超时;兜底块自身保持 nginx 默认值。([#5934])
|
||||
- **Docker:** 生产栈(`make up` / `scripts/deploy.sh`)现在可以在禁用 IPv6 的主机上启动。
|
||||
`docker/nginx/nginx.conf` 同时监听 `[::]:2026`;在以 `ipv6.disable=1` 启动的内核上,这条监听会让
|
||||
nginx 在启动时退出,容器因此反复重启,`make up` 永远无法就绪。开发用 compose 文件自 #2027 起
|
||||
@ -5232,3 +5239,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子
|
||||
[#5893]: https://github.com/bytedance/deer-flow/pull/5893
|
||||
[#5900]: https://github.com/bytedance/deer-flow/pull/5900
|
||||
[#5928]: https://github.com/bytedance/deer-flow/pull/5928
|
||||
[#5934]: https://github.com/bytedance/deer-flow/pull/5934
|
||||
|
||||
66
backend/tests/support/nginx_conf.py
Normal file
66
backend/tests/support/nginx_conf.py
Normal file
@ -0,0 +1,66 @@
|
||||
"""Helpers for asserting on the three maintained nginx configs.
|
||||
|
||||
The Docker production config, the local-dev config used by ``make dev`` and the
|
||||
Kubernetes/Helm ConfigMap template carry the same routing and must not drift;
|
||||
tests parametrize over ``NGINX_CONFIGS`` and inspect one ``location`` block at
|
||||
a time so a directive that merely appears elsewhere in the file cannot satisfy
|
||||
an assertion by accident.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[3]
|
||||
NGINX_CONFIGS = (
|
||||
"docker/nginx/nginx.conf",
|
||||
"docker/nginx/nginx.local.conf",
|
||||
"deploy/helm/deer-flow/templates/configmap-nginx.yaml",
|
||||
)
|
||||
|
||||
_SIZE_MULTIPLIERS = {"": 1, "k": 1024, "m": 1024**2, "g": 1024**3}
|
||||
_DURATION_MULTIPLIERS = {"": 1, "s": 1, "m": 60, "h": 3600}
|
||||
|
||||
|
||||
def read_config(path: str) -> str:
|
||||
return (REPO_ROOT / path).read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def extract_location_block(content: str, location_selector: str) -> str:
|
||||
"""Return the single ``location <location_selector> { ... }`` block.
|
||||
|
||||
Brace-depth matching keeps assertions inside that location, so the
|
||||
neighboring uploads location (which already has the large-body settings)
|
||||
cannot make another route's assertions pass.
|
||||
"""
|
||||
marker = re.compile(r"location\s+" + re.escape(location_selector) + r"\s*\{")
|
||||
match = marker.search(content)
|
||||
assert match, f"could not find `location {location_selector}` block"
|
||||
|
||||
start = match.end() - 1 # index of the opening brace
|
||||
depth = 0
|
||||
for i, ch in enumerate(content[start:], start=start):
|
||||
if ch == "{":
|
||||
depth += 1
|
||||
elif ch == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return content[start : i + 1]
|
||||
|
||||
raise AssertionError(f"unbalanced braces in `location {location_selector}` block")
|
||||
|
||||
|
||||
def parse_body_size_bytes(block: str) -> int:
|
||||
match = re.search(r"client_max_body_size\s+(\d+)\s*([mMkKgG]?)\s*;", block)
|
||||
assert match, "client_max_body_size value not found or not parseable"
|
||||
value, unit = match.groups()
|
||||
return int(value) * _SIZE_MULTIPLIERS[unit.lower()]
|
||||
|
||||
|
||||
def parse_read_timeout_seconds(block: str) -> int:
|
||||
# Anchored to the start of a line so a commented-out directive does not count.
|
||||
match = re.search(r"^\s*proxy_read_timeout\s+(\d+)([smh]?)\s*;", block, re.M)
|
||||
assert match, "no active proxy_read_timeout directive"
|
||||
value, unit = match.groups()
|
||||
return int(value) * _DURATION_MULTIPLIERS[unit]
|
||||
@ -30,17 +30,12 @@ used by ``make dev``, and the Kubernetes/Helm ConfigMap template.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
NGINX_CONFIGS = (
|
||||
"docker/nginx/nginx.conf",
|
||||
"docker/nginx/nginx.local.conf",
|
||||
"deploy/helm/deer-flow/templates/configmap-nginx.yaml",
|
||||
)
|
||||
from support.nginx_conf import NGINX_CONFIGS
|
||||
from support.nginx_conf import extract_location_block as _extract_location_block
|
||||
from support.nginx_conf import parse_body_size_bytes as _parse_body_size_bytes
|
||||
from support.nginx_conf import parse_read_timeout_seconds as _parse_read_timeout_seconds
|
||||
from support.nginx_conf import read_config as _read
|
||||
|
||||
# Text prompts never carry binary file attachments (those go through the
|
||||
# dedicated uploads route), so the ceiling here is intentionally well below
|
||||
@ -50,56 +45,9 @@ NGINX_CONFIGS = (
|
||||
_MIN_EXPECTED_BODY_SIZE_BYTES = 5 * 1024 * 1024
|
||||
_MAX_EXPECTED_BODY_SIZE_BYTES = 100 * 1024 * 1024
|
||||
|
||||
_SIZE_MULTIPLIERS = {"": 1, "k": 1024, "m": 1024**2, "g": 1024**3}
|
||||
|
||||
# The read timeout /api/langgraph/ already allows for a model-bound request.
|
||||
_MIN_BLOCKING_READ_TIMEOUT_SECONDS = 600
|
||||
|
||||
_DURATION_MULTIPLIERS = {"": 1, "s": 1, "m": 60, "h": 3600}
|
||||
|
||||
|
||||
def _read(path: str) -> str:
|
||||
return (REPO_ROOT / path).read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _extract_location_block(content: str, location_selector: str) -> str:
|
||||
"""Extract a single nginx ``location <location_selector> { ... }`` block
|
||||
by brace-depth matching, so assertions target only that location and
|
||||
can't be satisfied by a directive that merely appears elsewhere in the
|
||||
file (e.g. the neighboring uploads location, which already has both
|
||||
settings and must not make the langgraph-route assertions pass by
|
||||
accident)."""
|
||||
marker = re.compile(r"location\s+" + re.escape(location_selector) + r"\s*\{")
|
||||
match = marker.search(content)
|
||||
assert match, f"could not find `location {location_selector}` block"
|
||||
|
||||
start = match.end() - 1 # index of the opening brace
|
||||
depth = 0
|
||||
for i, ch in enumerate(content[start:], start=start):
|
||||
if ch == "{":
|
||||
depth += 1
|
||||
elif ch == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return content[start : i + 1]
|
||||
|
||||
raise AssertionError(f"unbalanced braces in `location {location_selector}` block")
|
||||
|
||||
|
||||
def _parse_body_size_bytes(block: str) -> int:
|
||||
match = re.search(r"client_max_body_size\s+(\d+)\s*([mMkKgG]?)\s*;", block)
|
||||
assert match, "client_max_body_size value not found or not parseable"
|
||||
value, unit = match.groups()
|
||||
return int(value) * _SIZE_MULTIPLIERS[unit.lower()]
|
||||
|
||||
|
||||
def _parse_read_timeout_seconds(block: str) -> int:
|
||||
# Anchored to the start of a line so a commented-out directive does not count.
|
||||
match = re.search(r"^\s*proxy_read_timeout\s+(\d+)([smh]?)\s*;", block, re.M)
|
||||
assert match, "no active proxy_read_timeout directive"
|
||||
value, unit = match.groups()
|
||||
return int(value) * _DURATION_MULTIPLIERS[unit]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", NGINX_CONFIGS)
|
||||
def test_langgraph_route_disables_request_buffering(path):
|
||||
|
||||
103
backend/tests/test_nginx_project_documents_body_size.py
Normal file
103
backend/tests/test_nginx_project_documents_body_size.py
Normal file
@ -0,0 +1,103 @@
|
||||
"""Project-document uploads through nginx must not be capped at nginx's 1m default.
|
||||
|
||||
``POST /api/projects/{project_id}/documents`` is a multipart upload that Gateway
|
||||
accepts up to ``uploads.max_file_size`` (50 MiB by default, the same limit as
|
||||
thread uploads). No nginx location matched it, so it fell through to the
|
||||
``/api/`` catch-all, which carries no ``client_max_body_size``: nginx answered
|
||||
a 2 MB PDF with a bare 413 before Gateway ever saw the request. Reproduced on
|
||||
``nginx:alpine`` with the shipped config -- the same body sent to
|
||||
``/api/threads/{id}/uploads`` passed nginx's size check.
|
||||
|
||||
The route needs its own location, with the uploads route's settings, in all
|
||||
three maintained configs.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
|
||||
import pytest
|
||||
from support.nginx_conf import NGINX_CONFIGS, extract_location_block, parse_body_size_bytes, parse_read_timeout_seconds, read_config
|
||||
|
||||
from app.gateway.routers.project_documents import router as project_documents_router
|
||||
from app.gateway.routers.uploads import DEFAULT_MAX_FILE_SIZE
|
||||
|
||||
PROJECT_DOCUMENTS_LOCATION = r"~ ^/api/projects/[^/]+/documents"
|
||||
UPLOADS_LOCATION = r"~ ^/api/threads/[^/]+/uploads"
|
||||
|
||||
# The /api/ catch-all these routes used to land on grants blocking Gateway calls
|
||||
# this much; moving them to their own location must not take it away.
|
||||
_MIN_READ_TIMEOUT_SECONDS = 600
|
||||
|
||||
|
||||
def _location_regex(selector: str) -> re.Pattern[str]:
|
||||
"""The PCRE nginx evaluates for a ``~`` location, as a Python pattern (identical for these anchors and classes)."""
|
||||
assert selector.startswith("~ ")
|
||||
return re.compile(selector[2:])
|
||||
|
||||
|
||||
def _router_paths() -> list[str]:
|
||||
"""Concrete request paths for every route the project-documents router serves."""
|
||||
paths = []
|
||||
for route in project_documents_router.routes:
|
||||
path = route.path.replace("{project_id}", "proj-42").replace("{document_id}", "doc-7").replace("{thread_id}", "thread-9")
|
||||
paths.append(path)
|
||||
assert paths, "project-documents router has no routes"
|
||||
return paths
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", NGINX_CONFIGS)
|
||||
def test_project_documents_route_allows_the_upload_limit_gateway_enforces(path):
|
||||
"""The location must admit at least what Gateway accepts, and stream it."""
|
||||
block = extract_location_block(read_config(path), PROJECT_DOCUMENTS_LOCATION)
|
||||
|
||||
size_bytes = parse_body_size_bytes(block)
|
||||
uploads_size_bytes = parse_body_size_bytes(extract_location_block(read_config(path), UPLOADS_LOCATION))
|
||||
|
||||
assert size_bytes >= DEFAULT_MAX_FILE_SIZE, f"{path}: project documents client_max_body_size is {size_bytes} bytes but Gateway accepts {DEFAULT_MAX_FILE_SIZE} by default; nginx would 413 a file Gateway allows"
|
||||
assert size_bytes == uploads_size_bytes, f"{path}: project documents and thread uploads share uploads.max_file_size, so their nginx ceilings must match"
|
||||
assert "proxy_request_buffering off;" in block, f"{path}: uploads must stream to Gateway instead of spooling to client_body_temp"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", NGINX_CONFIGS)
|
||||
def test_project_documents_route_keeps_the_catchall_read_timeout(path):
|
||||
"""attach-to-thread copies into a sandbox and content downloads stream; both used to get /api/'s 600s."""
|
||||
block = extract_location_block(read_config(path), PROJECT_DOCUMENTS_LOCATION)
|
||||
catchall = extract_location_block(read_config(path), "/api/")
|
||||
|
||||
assert parse_read_timeout_seconds(block) >= max(_MIN_READ_TIMEOUT_SECONDS, parse_read_timeout_seconds(catchall))
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", NGINX_CONFIGS)
|
||||
def test_project_documents_location_matches_every_router_path_and_nothing_else(path):
|
||||
"""The regex must cover the whole router (the upload is one of five routes) without swallowing the project routes above it."""
|
||||
content = read_config(path)
|
||||
extract_location_block(content, PROJECT_DOCUMENTS_LOCATION) # the selector under test is really in this file
|
||||
pattern = _location_regex(PROJECT_DOCUMENTS_LOCATION)
|
||||
|
||||
for request_path in _router_paths():
|
||||
assert pattern.search(request_path), f"{path}: {request_path} would fall through to /api/ and its 1m default"
|
||||
for other in ("/api/projects", "/api/projects/proj-42", "/api/projects/proj-42/threads", "/api/projectsx/documents", "/api/threads/t/uploads"):
|
||||
assert not pattern.search(other), f"{path}: {other} must not be routed as a project document"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", NGINX_CONFIGS)
|
||||
def test_project_documents_location_proxies_like_its_neighbours(path):
|
||||
"""Same upstream and forwarded headers as the uploads route, so only the size policy differs from /api/."""
|
||||
content = read_config(path)
|
||||
block = extract_location_block(content, PROJECT_DOCUMENTS_LOCATION)
|
||||
uploads = extract_location_block(content, UPLOADS_LOCATION)
|
||||
|
||||
def directives(text: str) -> set[str]:
|
||||
return {line.strip() for line in text.splitlines() if re.match(r"\s*(proxy_pass|proxy_http_version|proxy_set_header)\b", line)}
|
||||
|
||||
assert directives(block) == directives(uploads)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", NGINX_CONFIGS)
|
||||
def test_api_catchall_still_keeps_nginx_default_body_policy(path):
|
||||
"""The fix is a dedicated location, not a blanket raise on /api/ (auth, config writes and JSON routes stay at the default)."""
|
||||
block = extract_location_block(read_config(path), "/api/")
|
||||
|
||||
assert "client_max_body_size" not in block
|
||||
assert "proxy_request_buffering" not in block
|
||||
@ -160,6 +160,23 @@ data:
|
||||
proxy_request_buffering off;
|
||||
}
|
||||
|
||||
# Project documents — multipart upload plus the promote/attach/content/
|
||||
# delete routes. Without its own location the upload fell through to
|
||||
# /api/ and nginx's 1m default returned 413 before Gateway saw it,
|
||||
# although Gateway allows uploads.max_file_size (50 MiB by default).
|
||||
# Same ceiling as uploads; the read timeout keeps what /api/ granted.
|
||||
location ~ ^/api/projects/[^/]+/documents {
|
||||
proxy_pass http://gateway_upstream;
|
||||
proxy_http_version 1.1;
|
||||
proxy_set_header Host $http_host;
|
||||
proxy_set_header X-Real-IP $remote_addr;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
proxy_set_header X-Forwarded-Proto $forwarded_proto;
|
||||
client_max_body_size 100M;
|
||||
proxy_request_buffering off;
|
||||
proxy_read_timeout 600s;
|
||||
}
|
||||
|
||||
location ~ ^/api/threads {
|
||||
proxy_pass http://gateway_upstream;
|
||||
proxy_http_version 1.1;
|
||||
|
||||
@ -201,6 +201,26 @@ http {
|
||||
# Disable response buffering to avoid permission errors
|
||||
}
|
||||
|
||||
# Custom API: Project documents (multipart upload, promote-from-thread,
|
||||
# attach-to-thread, content download, delete). Without its own location
|
||||
# the upload fell through to /api/ and nginx's 1m default rejected
|
||||
# anything larger with a 413 before Gateway saw it, although Gateway
|
||||
# allows uploads.max_file_size (50 MiB by default) on this route. Same
|
||||
# ceiling and streaming settings as the thread uploads route above; the
|
||||
# read timeout keeps what /api/ already granted these routes.
|
||||
location ~ ^/api/projects/[^/]+/documents {
|
||||
proxy_pass http://$gateway_upstream;
|
||||
proxy_http_version 1.1;
|
||||
proxy_set_header Host $http_host;
|
||||
proxy_set_header X-Real-IP $remote_addr;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
proxy_set_header X-Forwarded-Proto $forwarded_proto;
|
||||
|
||||
client_max_body_size 100M;
|
||||
proxy_request_buffering off;
|
||||
proxy_read_timeout 600s;
|
||||
}
|
||||
|
||||
# Live browser stream is a WebSocket upgrade. It must be matched before
|
||||
# the generic /api/threads regex below (which omits Upgrade/Connection
|
||||
# forwarding and would downgrade it to plain HTTP).
|
||||
|
||||
@ -206,6 +206,30 @@ http {
|
||||
proxy_cache off;
|
||||
}
|
||||
|
||||
# Custom API: Project documents (multipart upload, promote-from-thread,
|
||||
# attach-to-thread, content download, delete). Without its own location
|
||||
# the upload fell through to /api/ and nginx's 1m default rejected
|
||||
# anything larger with a 413 before Gateway saw it, although Gateway
|
||||
# allows uploads.max_file_size (50 MiB by default) on this route. Same
|
||||
# ceiling and streaming settings as the thread uploads route above; the
|
||||
# read timeout keeps what /api/ already granted these routes.
|
||||
location ~ ^/api/projects/[^/]+/documents {
|
||||
proxy_pass http://gateway;
|
||||
proxy_http_version 1.1;
|
||||
proxy_set_header Host $http_host;
|
||||
proxy_set_header X-Real-IP $remote_addr;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
proxy_set_header X-Forwarded-Proto $scheme;
|
||||
|
||||
client_max_body_size 100M;
|
||||
proxy_request_buffering off;
|
||||
proxy_read_timeout 600s;
|
||||
|
||||
# Disable response buffering to avoid permission errors
|
||||
proxy_buffering off;
|
||||
proxy_cache off;
|
||||
}
|
||||
|
||||
# Live browser stream is a WebSocket upgrade. It must be matched before
|
||||
# the generic /api/threads regex below (which omits Upgrade/Connection
|
||||
# forwarding and would downgrade it to plain HTTP).
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user