Fix OTC order create JSON validation by minyanyi · Pull Request #5582 · Scottcjn/Rustchain

minyanyi · 2026-05-17T16:52:44Z

Summary

reject non-object JSON bodies on POST /api/orders with a stable JSON 400
parse ttl_seconds through a validation helper so malformed values return JSON 400 instead of Flask HTML 500
keep valid buy-order creation and TTL clamping behavior intact

Validation

python -m pytest tests/test_otc_bridge_query_validation.py -q -> 12 passed
python -m py_compile otc-bridge\otc_bridge.py tests\test_otc_bridge_query_validation.py
git diff --check

Additional note: after installing the missing local flask-cors test dependency, python -m pytest otc-bridge/test_otc_bridge.py tests/test_otc_bridge_query_validation.py -q reached collection and ran, with 36 passed and 2 existing failures in unrelated order confirmation/HTLC tests (test_confirm_matched_order, test_legacy_money_schema_accepts_new_orders_and_trades).

github-actions · 2026-05-17T16:52:51Z

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

Non-doc PRs have a BCOS-L1 or BCOS-L2 label
Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
New code files include an SPDX license header
You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

iamdinhthuan

I checked this at head 7fa276495d7499d830b4d4f78a7442c381fc2938. The main string/non-object cases are improved, and the focused suite passes, but ttl_seconds validation still accepts JSON booleans as valid TTL input.

Because Python bool is a subclass of int, parse_ttl_seconds() currently does int(True) / int(False), clamps it to 3600, and creates the order. JSON booleans are not integer TTL values, so this still leaves malformed client input accepted instead of returning the stable JSON 400 promised by #5525.

Reproduction on the PR head after init_db() with a Flask test client:

payload={"side":"buy","pair":"RTC/USDC","wallet":"buyer-1","amount_rtc":1,"price_per_rtc":"0.10","ttl_seconds":true}
POST /api/orders -> 201
body["expires_in_hours"] == 1.0

payload={"side":"buy","pair":"RTC/USDC","wallet":"buyer-1","amount_rtc":1,"price_per_rtc":"0.10","ttl_seconds":false}
POST /api/orders -> 201
body["expires_in_hours"] == 1.0

Expected behavior: reject boolean ttl_seconds with the same JSON 400 class as other non-integer TTL values, e.g. {"error":"ttl_seconds must be an integer"}.

Validation I ran:

python3 -B -m py_compile otc-bridge/otc_bridge.py tests/test_otc_bridge_query_validation.py
uv run --no-project --with pytest --with flask --with flask-cors python -B -m pytest -q tests/test_otc_bridge_query_validation.py --noconftest
# 12 passed
git diff --check origin/main...HEAD
python3 tools/bcos_spdx_check.py --base-ref origin/main
# BCOS SPDX check: OK

The fix should be small: reject isinstance(raw_value, bool) before calling int(raw_value), and add regression coverage for true and false TTL JSON values.

508704820 · 2026-05-17T16:56:55Z

Security Review ✅

OTC order create JSON validation. Same class as #5534/#5551 — validates JSON body type and field types before order creation logic.

Reviewed by Xeophon - Solana: Lt9nERv6VHsojw15LpFeiaabuphAggzfLF9sM9UXRrZ

HCIE2054

LGTM!

Signed-off-by: minyanyi <192839474+minyanyi@users.noreply.github.com>

minyanyi · 2026-05-17T17:00:09Z

Updated the PR to tighten the ttl_seconds validator after the review note about float truncation.

Additional covered invalid TTL shapes now return the same JSON 400 response instead of being truncated or coerced:

"1.5"
1.5
true

Fresh validation after the update:

python -m pytest tests\test_otc_bridge_query_validation.py -q -> 12 passed
python -m py_compile otc-bridge\otc_bridge.py tests\test_otc_bridge_query_validation.py
git diff --check

iamdinhthuan

Rechecked the updated head 30a28838076e4b0230217c7186b06ee0fb9a2d85. The boolean TTL blocker from my earlier review is fixed: direct probes now return 400 for ttl_seconds: true, ttl_seconds: false, and ttl_seconds: 3600.5, while a valid integer string still creates the order.

Validation run on the updated head:

python3 -B -m py_compile otc-bridge/otc_bridge.py tests/test_otc_bridge_query_validation.py
uv run --no-project --with pytest --with flask --with flask-cors python -B -m pytest -q tests/test_otc_bridge_query_validation.py --noconftest
# 12 passed
git diff --check origin/main...HEAD
python3 tools/bcos_spdx_check.py --base-ref origin/main
# BCOS SPDX check: OK

No additional bounty claim for this follow-up; this just resolves the previous changes-requested finding.

ZacharyZhang-NY

Reviewed PR #5582 at head 30a2883.

Approved. The create-order path now handles missing JSON, non-object JSON, and invalid ttl_seconds shapes before the order creation logic. The updated TTL parser rejects booleans, floats, and non-integer strings while preserving valid integer-string TTL input and the existing min/max clamp behavior.

Validation run:

git diff --check origin/main...HEAD -- otc-bridge/otc_bridge.py tests/test_otc_bridge_query_validation.py
python -B -m py_compile otc-bridge/otc_bridge.py tests/test_otc_bridge_query_validation.py
python -B -m pytest -q tests/test_otc_bridge_query_validation.py --tb=short

Result: diff check clean, py_compile clean, OTC bridge query validation tests 12 passed.

TJCurnutte

Approved. I re-tested the OTC order-create validation path at head 30a28838076e4b0230217c7186b06ee0fb9a2d85.

Validation run:

git diff --check origin/main...HEAD -- otc-bridge/otc_bridge.py tests/test_otc_bridge_query_validation.py
python3 -B -m py_compile otc-bridge/otc_bridge.py tests/test_otc_bridge_query_validation.py
uv run --no-project --with pytest --with flask --with flask-cors --with requests python -B -m pytest -q -o addopts='' --noconftest tests/test_otc_bridge_query_validation.py
uv run --no-project --with flask --with flask-cors --with requests python <focused Flask temp-DB probe>

Results:

git diff --check passed.
py_compile passed.
Focused pytest passed: 12 passed in 0.54s.
Runtime probe initialized a fresh OTC SQLite DB and confirmed:
- top-level JSON array body returns 400 with {"error": "JSON object required"};
- ttl_seconds values "abc", "1.5", 1.5, and true all return 400 with {"error": "ttl_seconds must be an integer"};
- valid integer and string-integer TTLs (7200 / "7200") still create orders with 201, ok: true, and expires_in_hours: 2.0.

That covers the previous failure mode here: malformed create-order JSON no longer reaches the route internals as a list, and TTL float/bool/string coercion no longer silently creates orders. The valid order path still works. This is good to merge from my review.

minyanyi · 2026-05-17T17:34:30Z

CI follow-up: I opened #5587 to fix the current blocking pytest tests/ collection failure caused by missing test dependencies (aiohttp, flask_cors, and matplotlib). The red CI here appears to stop before this PR's focused OTC tests can run.

Focused validation after the latest TTL tightening update remains: python -m pytest tests\test_otc_bridge_query_validation.py -q -> 12 passed, plus python -m py_compile otc-bridge\otc_bridge.py tests\test_otc_bridge_query_validation.py and git diff --check.

jaxint

LGTM! Great work on this PR. 🚀

HCIE2054

LGTM!

kekehanshujun

I reviewed the OTC order validation change. The non-object JSON guard prevents list/scalar payloads from reaching .get(), and the new parse_ttl_seconds() path rejects non-integer TTL values, including bools, before clamping valid values into the supported range.

The added tests cover non-object bodies, invalid TTL types/strings, and a valid string TTL. I do not see a blocker for this fix.

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

Scottcjn · 2026-05-18T14:36:02Z

Duplicate of #5527 by @ZacharyZhang-NY for OTC order JSON/TTL validation. Note: canonical fix already merged in weilixiong#5526. Codex batch 2 Q5.

github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) tests Test suite changes size/M PR: 51-200 lines labels May 17, 2026

minyanyi mentioned this pull request May 17, 2026

Bug: OTC order creation 500s on non-object JSON and invalid ttl_seconds #5525

Closed

iamdinhthuan suggested changes May 17, 2026

View reviewed changes

iamdinhthuan mentioned this pull request May 17, 2026

[Bounty Claim] PR Review - RustChain PR #5582 boolean ttl_seconds validation (Bounty #73) Scottcjn/rustchain-bounties#10661

Open

HCIE2054 approved these changes May 17, 2026

View reviewed changes

Fix OTC order create JSON validation

30a2883

Signed-off-by: minyanyi <192839474+minyanyi@users.noreply.github.com>

minyanyi force-pushed the codex/otc-order-validation-5525 branch from 7fa2764 to 30a2883 Compare May 17, 2026 16:59

iamdinhthuan approved these changes May 17, 2026

View reviewed changes

ZacharyZhang-NY approved these changes May 17, 2026

View reviewed changes

This was referenced May 17, 2026

[Review Bounty Claim] PR #5582 OTC order JSON validation Scottcjn/rustchain-bounties#10669

Open

[Bounty Claim Consolidation] ZacharyZhang-NY PR Reviews for Bounty #73 Scottcjn/rustchain-bounties#10608

Open

TJCurnutte approved these changes May 17, 2026

View reviewed changes

TJCurnutte mentioned this pull request May 17, 2026

[BOUNTY] Code Review Bounty Program — Review PRs, Earn RTC (100 RTC Pool) Scottcjn/rustchain-bounties#73

Open

jaxint approved these changes May 17, 2026

View reviewed changes

jaxint mentioned this pull request May 17, 2026

PR Review Claim - 40 PRs - 2026-05-17 18:08 UTC Scottcjn/rustchain-bounties#10712

Open

HCIE2054 approved these changes May 17, 2026

View reviewed changes

kekehanshujun approved these changes May 17, 2026

View reviewed changes

This was referenced May 17, 2026

[Bounty Claim] RustChain review batch - payout recovery, UTXO, OTC, CI deps Scottcjn/rustchain-bounties#10750

Open

fix: validate OTC order JSON body and ttl_seconds #5551

Closed

HCIE2054 approved these changes May 17, 2026

View reviewed changes

HCIE2054 mentioned this pull request May 17, 2026

PR Review Batch - 100 PRs - RTC Scottcjn/rustchain-bounties#10784

Open

HCIE2054 approved these changes May 17, 2026

View reviewed changes

HCIE2054 approved these changes May 18, 2026

View reviewed changes

Scottcjn closed this May 18, 2026

Uh oh!

Conversation

minyanyi commented May 17, 2026

Summary

Validation

Uh oh!

github-actions Bot commented May 17, 2026

Uh oh!

iamdinhthuan left a comment

Choose a reason for hiding this comment

Uh oh!

508704820 commented May 17, 2026

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

minyanyi commented May 17, 2026

Uh oh!

iamdinhthuan left a comment

Choose a reason for hiding this comment

Uh oh!

ZacharyZhang-NY left a comment

Choose a reason for hiding this comment

Uh oh!

TJCurnutte left a comment

Choose a reason for hiding this comment

Uh oh!

minyanyi commented May 17, 2026

Uh oh!

jaxint left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

kekehanshujun left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

Scottcjn commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants