Skip to content

Decode list-query parameters so encoded filters match rows #336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tylerreckart merged 5 commits into from
Sep 21, 2026
Merged

Decode list-query parameters so encoded filters match rows #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7ed3d0e
Decode list-query parameters through parse_query
cursoragent Sep 13, 2026
59532fb
Merge origin/main into fix/list-query-url-decode
mnaumanofficial13-svg Sep 21, 2026
87a2c81
Merge origin/main into fix/list-query-url-decode
mnaumanofficial13-svg Sep 21, 2026
c907634
Merge origin/main into fix/list-query-url-decode
mnaumanofficial13-svg Sep 21, 2026
47e9476
Merge origin/main into fix/list-query-url-decode
mnaumanofficial13-svg Sep 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ loosely while pre-1.0 (breaking changes can land on minor bumps).
## [Unreleased]

### Fixed
- **List-query URL decoding.** Lesson, todo, schedule, request, run,
and admin-audit list handlers now go through `parse_query()` (same as
conversation list). Encoded spaces (`q=rate%20limit`, `+`) and
percent-encoded filter values no longer miss rows.
- **Compaction boundary vs user `QUERY:` / `[` text.**
`strip_compaction_preambles` now takes the first orchestrator
`\n\nQUERY: ` wrapper (not the last) and strips only known
Expand Down
11 changes: 11 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1332,6 +1332,17 @@ if(INDEX_BUILD_TESTS)
)
add_test(NAME unit_cors COMMAND unit_cors)

# --- Unit tests for query-string decode (list handlers share parse_query) ---
add_executable(unit_http_util
tests/test_http_util.cpp
src/api/http_util.cpp
)
target_include_directories(unit_http_util PRIVATE
${CMAKE_SOURCE_DIR}/include
${CMAKE_SOURCE_DIR}/third_party/doctest
)
add_test(NAME unit_http_util COMMAND unit_http_util)

add_executable(unit_sse_mailbox tests/test_sse_mailbox.cpp)
target_include_directories(unit_sse_mailbox PRIVATE
${CMAKE_SOURCE_DIR}/include
Expand Down
177 changes: 45 additions & 132 deletions src/api_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,26 +331,12 @@ void handle_admin(int fd, const HttpRequest& req,
}
int64_t before_id = 0;
int limit = 50;
auto qpos = req.path.find('?');
if (qpos != std::string::npos) {
std::string qs = req.path.substr(qpos + 1);
size_t i = 0;
while (i < qs.size()) {
size_t amp = qs.find('&', i);
std::string pair = qs.substr(i, amp - i);
auto eq = pair.find('=');
if (eq != std::string::npos) {
std::string k = pair.substr(0, eq);
std::string v = pair.substr(eq + 1);
if (k == "before_id") {
try { before_id = std::stoll(v); } catch (...) {}
} else if (k == "limit") {
try { limit = std::stoi(v); } catch (...) {}
}
}
if (amp == std::string::npos) break;
i = amp + 1;
}
const auto qp = parse_query(req.path);
if (auto it = qp.find("before_id"); it != qp.end()) {
try { before_id = std::stoll(it->second); } catch (...) {}
}
if (auto it = qp.find("limit"); it != qp.end()) {
try { limit = std::stoi(it->second); } catch (...) {}
}
auto rows = tenants.list_admin_audit(before_id, limit);
auto arr = jarr();
Expand Down Expand Up @@ -3608,22 +3594,9 @@ request_status_to_json(const TenantStore::RequestStatus& s) {
void handle_request_list(int fd, const HttpRequest& req,
TenantStore& tenants, const Tenant& tenant) {
int limit = 100;
auto qpos = req.path.find('?');
if (qpos != std::string::npos) {
std::string qs = req.path.substr(qpos + 1);
size_t i = 0;
while (i < qs.size()) {
size_t amp = qs.find('&', i);
std::string pair = qs.substr(i, amp - i);
auto eq = pair.find('=');
if (eq != std::string::npos) {
std::string k = pair.substr(0, eq);
std::string v = pair.substr(eq + 1);
if (k == "limit") try { limit = std::stoi(v); } catch (...) {}
}
if (amp == std::string::npos) break;
i = amp + 1;
}
const auto qp = parse_query(req.path);
if (auto it = qp.find("limit"); it != qp.end()) {
try { limit = std::stoi(it->second); } catch (...) {}
}
auto rows = tenants.list_request_status(tenant.id, limit);
auto arr = jarr();
Expand Down Expand Up @@ -3690,22 +3663,9 @@ void handle_request_events(int fd, const std::string& request_id,
}

int64_t since_seq = 0;
auto qpos = req.path.find('?');
if (qpos != std::string::npos) {
std::string qs = req.path.substr(qpos + 1);
size_t i = 0;
while (i < qs.size()) {
size_t amp = qs.find('&', i);
std::string pair = qs.substr(i, amp - i);
auto eq = pair.find('=');
if (eq != std::string::npos) {
std::string k = pair.substr(0, eq);
std::string v = pair.substr(eq + 1);
if (k == "since_seq") try { since_seq = std::stoll(v); } catch (...) {}
}
if (amp == std::string::npos) break;
i = amp + 1;
}
const auto qp = parse_query(req.path);
if (auto it = qp.find("since_seq"); it != qp.end()) {
try { since_seq = std::stoll(it->second); } catch (...) {}
}

// Open the SSE response. CORS headers + X-Accel-Buffering: no so
Expand Down Expand Up @@ -3929,24 +3889,14 @@ void handle_lesson_list(int fd, const HttpRequest& req,
std::string agent_id;
std::string query;
int limit = 100;
auto qpos = req.path.find('?');
if (qpos != std::string::npos) {
std::string qs = req.path.substr(qpos + 1);
size_t i = 0;
while (i < qs.size()) {
size_t amp = qs.find('&', i);
std::string pair = qs.substr(i, amp - i);
auto eq = pair.find('=');
if (eq != std::string::npos) {
std::string k = pair.substr(0, eq);
std::string v = pair.substr(eq + 1);
if (k == "agent_id") agent_id = v;
else if (k == "q") query = v;
else if (k == "limit") try { limit = std::stoi(v); } catch (...) {}
}
if (amp == std::string::npos) break;
i = amp + 1;
}
// parse_query url-decodes — the previous hand-rolled split left
// `q=rate%20limit` as a literal, so standard clients missed rows
// that conversation list (already on parse_query) would find.
const auto qp = parse_query(req.path);
if (auto it = qp.find("agent_id"); it != qp.end()) agent_id = it->second;
if (auto it = qp.find("q"); it != qp.end()) query = it->second;
if (auto it = qp.find("limit"); it != qp.end()) {
try { limit = std::stoi(it->second); } catch (...) {}
}
auto rows = query.empty()
? tenants.list_lessons(tenant.id, agent_id, limit)
Expand Down Expand Up @@ -4114,33 +4064,22 @@ void handle_todo_list(int fd, const HttpRequest& req,
TenantStore& tenants, const Tenant& tenant) {
TenantStore::TodoFilter f;
f.limit = 200;
auto qpos = req.path.find('?');
if (qpos != std::string::npos) {
std::string qs = req.path.substr(qpos + 1);
size_t i = 0;
while (i < qs.size()) {
size_t amp = qs.find('&', i);
std::string pair = qs.substr(i, amp - i);
auto eq = pair.find('=');
if (eq != std::string::npos) {
std::string k = pair.substr(0, eq);
std::string v = pair.substr(eq + 1);
if (k == "conversation_id") {
// `conversation_id=tenant` is the spelled-out form
// for the unscoped-only filter; numeric values pass
// through directly (positive = OR-NULL fallback to
// unscoped, 0 = no filter, negative = unscoped only).
if (v == "tenant" || v == "unscoped") f.conversation_id = -1;
else try { f.conversation_id = std::stoll(v); } catch (...) {}
}
else if (k == "status") f.status_filter = v;
else if (k == "agent_id") f.agent_id_filter = v;
else if (k == "limit")
try { f.limit = std::stoi(v); } catch (...) {}
}
if (amp == std::string::npos) break;
i = amp + 1;
}
const auto qp = parse_query(req.path);
if (auto it = qp.find("conversation_id"); it != qp.end()) {
const std::string& v = it->second;
// `conversation_id=tenant` is the spelled-out form
// for the unscoped-only filter; numeric values pass
// through directly (positive = OR-NULL fallback to
// unscoped, 0 = no filter, negative = unscoped only).
if (v == "tenant" || v == "unscoped") f.conversation_id = -1;
else try { f.conversation_id = std::stoll(v); } catch (...) {}
}
if (auto it = qp.find("status"); it != qp.end())
f.status_filter = it->second;
if (auto it = qp.find("agent_id"); it != qp.end())
f.agent_id_filter = it->second;
if (auto it = qp.find("limit"); it != qp.end()) {
try { f.limit = std::stoi(it->second); } catch (...) {}
}
// Reject unknown / TUI-origin conversation ids. Clearing the filter
// would return tenant-wide todos and leak TUI-scoped rows over HTTP.
Expand Down Expand Up @@ -4497,24 +4436,9 @@ void handle_schedule_create(int fd, const HttpRequest& req,
void handle_schedule_list(int fd, const HttpRequest& req,
TenantStore& tenants, const Tenant& tenant) {
std::string status_filter;
auto qpos = req.path.find('?');
if (qpos != std::string::npos) {
std::string qs = req.path.substr(qpos + 1);
// Tiny query-string parser — only one key we look at.
size_t i = 0;
while (i < qs.size()) {
size_t amp = qs.find('&', i);
std::string pair = qs.substr(i, amp - i);
auto eq = pair.find('=');
if (eq != std::string::npos) {
std::string k = pair.substr(0, eq);
std::string v = pair.substr(eq + 1);
if (k == "status") status_filter = v;
}
if (amp == std::string::npos) break;
i = amp + 1;
}
}
const auto qp = parse_query(req.path);
if (auto it = qp.find("status"); it != qp.end())
status_filter = it->second;
auto rows = tenants.list_scheduled_tasks(tenant.id, status_filter, /*limit=*/200);
auto arr = jarr();
for (const auto& r : rows) arr->as_array_mut().push_back(scheduled_task_to_json(r));
Expand Down Expand Up @@ -5581,23 +5505,12 @@ void handle_runs_list(int fd, const HttpRequest& req,
TenantStore& tenants, const Tenant& tenant) {
int64_t since = 0;
int64_t task_id = 0;
auto qpos = req.path.find('?');
if (qpos != std::string::npos) {
std::string qs = req.path.substr(qpos + 1);
size_t i = 0;
while (i < qs.size()) {
size_t amp = qs.find('&', i);
std::string pair = qs.substr(i, amp - i);
auto eq = pair.find('=');
if (eq != std::string::npos) {
std::string k = pair.substr(0, eq);
std::string v = pair.substr(eq + 1);
if (k == "since") try { since = std::stoll(v); } catch (...) {}
if (k == "task_id") try { task_id = std::stoll(v); } catch (...) {}
}
if (amp == std::string::npos) break;
i = amp + 1;
}
const auto qp = parse_query(req.path);
if (auto it = qp.find("since"); it != qp.end()) {
try { since = std::stoll(it->second); } catch (...) {}
}
if (auto it = qp.find("task_id"); it != qp.end()) {
try { task_id = std::stoll(it->second); } catch (...) {}
}
auto runs = tenants.list_task_runs(tenant.id, task_id, since, /*limit=*/200);
auto arr = jarr();
Expand Down
53 changes: 53 additions & 0 deletions tests/test_http_util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// tests/test_http_util.cpp — query-string decode used by list handlers
#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include "doctest.h"
#include "api/http_util.h"

using namespace arbiter;

TEST_CASE("url_decode turns percent-escapes and plus into the raw value") {
CHECK(url_decode("rate%20limit") == "rate limit");
CHECK(url_decode("rate+limit") == "rate limit");
CHECK(url_decode("in%5Fprogress") == "in_progress");
CHECK(url_decode("plain") == "plain");
CHECK(url_decode("%2Ftmp") == "/tmp");
// Invalid % sequences stay literal (same as the previous splitters).
CHECK(url_decode("100%") == "100%");
CHECK(url_decode("%GG") == "%GG");
}

TEST_CASE("parse_query decodes list-handler filters the hand-rolled split missed") {
// GET /v1/lessons?q=rate%20limit — browsers and curl --data-urlencode
// send this; the old pair-splitter searched for the literal "%20".
auto lessons = parse_query("/v1/lessons?agent_id=scout&q=rate%20limit&limit=20");
CHECK(lessons["agent_id"] == "scout");
CHECK(lessons["q"] == "rate limit");
CHECK(lessons["limit"] == "20");

auto plus = parse_query("/v1/lessons?q=rate+limit");
CHECK(plus["q"] == "rate limit");

auto todos = parse_query("/v1/todos?status=in%5Fprogress&conversation_id=tenant");
CHECK(todos["status"] == "in_progress");
CHECK(todos["conversation_id"] == "tenant");

auto schedules = parse_query("/v1/schedules?status=active");
CHECK(schedules["status"] == "active");

// Numeric filters: encoded digits must still parse with stoll/stoi.
auto audit = parse_query("/v1/admin/audit?before_id=%31%30&limit=%35%30");
CHECK(audit["before_id"] == "10");
CHECK(audit["limit"] == "50");

auto encoded_key = parse_query("/v1/lessons?q%3D=nope&q=ok");
CHECK(encoded_key["q"] == "ok");
CHECK(encoded_key["q="] == "nope");
}

TEST_CASE("parse_query last value wins and ignores valueless pairs") {
auto qp = parse_query("/v1/lessons?q=first&q=second&flag&limit=5");
CHECK(qp["q"] == "second");
CHECK(qp["limit"] == "5");
CHECK(qp.count("flag") == 0);
CHECK(parse_query("/v1/lessons").empty());
}
Loading