Skip to content

Commit 9a21bc4

Browse files
therealalephclaude
andcommitted
fix: v1.9.12 — gate parallel_relay fan-out to idempotent methods only (#743)
Reported in #743: with `parallel_relay > 1`, a single POST (e.g. submitting a comment) reached the destination as N concurrent requests, so the comment got posted twice. Root cause is unfixable from the Rust side: `select_ok` cancels only OUR futures, but Apps Script has no way to learn the cancellation, so every fan-out call still runs to completion and each `UrlFetchApp.fetch()` still hits the destination. Fan-out now only triggers for idempotent methods (GET / HEAD / OPTIONS); POST / PUT / PATCH / DELETE always go sequential. Same pattern as `SAFE_REPLAY_METHODS` in Code.gs `_doBatch` fallback — safe methods are idempotent so re-firing is at worst wasteful, unsafe methods can have side effects so re-firing is incorrect. New regression test locks down `is_method_safe_for_fanout` predicate. Tests: 180 lib + 35 tunnel-node green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c2a33a8 commit 9a21bc4

4 files changed

Lines changed: 72 additions & 3 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "mhrv-rs"
3-
version = "1.9.11"
3+
version = "1.9.12"
44
edition = "2021"
55
description = "Rust port of MasterHttpRelayVPN -- DPI bypass via Google Apps Script relay with domain fronting"
66
license = "MIT"

docs/changelog/v1.9.12.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
<!-- see docs/changelog/v1.1.0.md for the file format: Persian, then `---`, then English. -->
2+
• Fix `parallel_relay` causing duplicate POSTs ([#743](https://github.com/therealaleph/MasterHttpRelayVPN-RUST/issues/743)): وقتی `parallel_relay > 1` تنظیم بود، تک‌POST کاربر (مثل ارسال کامنت در GitHub) به‌عنوان دو/چند درخواست همزمان به سایت مقصد می‌رسید + کامنت دو بار ثبت می‌شد. علت: `select_ok` فقط future‌های Rust سمت ما را cancel می‌کنه، ولی Apps Script سرور‌سایده هیچ خبری از cancel ندارد — هر فراخوانی fan-out روی Apps Script کامل می‌شه و `UrlFetchApp.fetch()` هر کدام به مقصد می‌رسه. حالا fan-out فقط برای متدهای **idempotent** (GET / HEAD / OPTIONS) اجرا می‌شه؛ POST / PUT / PATCH / DELETE همیشه sequential می‌رن — کاربر روی browse کاهش tail latency رو نگه می‌داره و روی form submit از duplicate side-effect ایمن می‌مونه. الگوی همان `SAFE_REPLAY_METHODS` که در `Code.gs` `_doBatch` fallback داریم. تست regression جدید locks down predicate. **۱۸۰ lib + ۳۵ tunnel-node test** همه pass.
3+
---
4+
• Fix `parallel_relay` causing duplicate POSTs ([#743](https://github.com/therealaleph/MasterHttpRelayVPN-RUST/issues/743)): with `parallel_relay > 1` set, a single user POST (e.g. submitting a comment on GitHub) was reaching the destination as two or more concurrent requests, so the comment got posted twice. Root cause: `select_ok` only cancels the loser futures on our side, but Apps Script has no way to learn about that cancellation server-side, so every fan-out call still runs to completion and each `UrlFetchApp.fetch()` to the destination still fires. Fan-out now only triggers for **idempotent** methods (GET / HEAD / OPTIONS); POST / PUT / PATCH / DELETE always go sequential — users keep the p95 tail-latency win on browsing without losing correctness on form submits. Same pattern as the `SAFE_REPLAY_METHODS` guard in `Code.gs` `_doBatch` fallback. New regression test locks down the predicate. **180 lib + 35 tunnel-node tests passing.**

src/domain_fronter.rs

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1169,8 +1169,29 @@ impl DomainFronter {
11691169
// Fan-out path: fire N instances in parallel, return first Ok, cancel
11701170
// the rest. Clamps to number of available script IDs so the single-ID
11711171
// case is a no-op even if parallel_relay>1 was configured.
1172+
//
1173+
// `select_ok` cancels the loser futures, but those futures only own
1174+
// the OUR-side I/O (TLS write, response read) — the Apps Script
1175+
// server has no idea the racing Rust task is gone, so every fan-out
1176+
// call still completes server-side and Apps Script's
1177+
// `UrlFetchApp.fetch()` to the destination still fires. For
1178+
// **non-idempotent** methods (POST / PUT / PATCH / DELETE) this
1179+
// surfaces as duplicate writes at the destination — a comment
1180+
// posted twice, a vote double-counted, a payment double-charged.
1181+
//
1182+
// Reported in #743: parallel_relay=2 + a POST to GitHub created
1183+
// two issue comments per submission. Same root cause as the
1184+
// SAFE_REPLAY_METHODS guard in Code.gs's `_doBatch` fallback —
1185+
// safe methods are idempotent, so re-firing is at worst wasteful;
1186+
// unsafe methods can have side effects, so re-firing is incorrect.
1187+
//
1188+
// Drop to sequential for non-idempotent methods regardless of
1189+
// `parallel_relay` setting. Users keep p95 wins on browsing /
1190+
// GET-heavy traffic (the common case) and don't lose correctness
1191+
// on form submits.
1192+
let method_safe_for_fanout = is_method_safe_for_fanout(method);
11721193
let fan = self.parallel_relay.min(self.script_ids.len()).max(1);
1173-
if fan >= 2 {
1194+
if fan >= 2 && method_safe_for_fanout {
11741195
return self.do_relay_parallel(method, url, headers, body, fan).await;
11751196
}
11761197

@@ -2697,6 +2718,18 @@ fn parse_status_line(line: &str) -> Result<u16, FronterError> {
26972718
code.parse::<u16>().map_err(|_| FronterError::BadResponse(format!("bad status code: {}", code)))
26982719
}
26992720

2721+
/// Returns `true` if the HTTP method is safe to fan-out across multiple
2722+
/// Apps Script deployments (i.e. idempotent per RFC 9110 §9.2.2). Used
2723+
/// by `do_relay_with_retry` to gate the `parallel_relay` fan-out so that
2724+
/// non-idempotent operations (POST / PUT / PATCH / DELETE) don't double-
2725+
/// fire at the destination — Apps Script `UrlFetchApp.fetch()` can't be
2726+
/// cancelled mid-request from our side, so every parallel attempt
2727+
/// completes server-side even when our `select_ok` already returned a
2728+
/// winner. See #743 for the user-visible bug (duplicate POSTs).
2729+
fn is_method_safe_for_fanout(method: &str) -> bool {
2730+
matches!(method.to_ascii_uppercase().as_str(), "GET" | "HEAD" | "OPTIONS")
2731+
}
2732+
27002733
/// Parse the JSON envelope from Apps Script and build a raw HTTP response.
27012734
fn parse_relay_json(body: &[u8]) -> Result<Vec<u8>, FronterError> {
27022735
let text = std::str::from_utf8(body)
@@ -3591,6 +3624,38 @@ hello";
35913624
assert_eq!(mask_script_id("AKfycbx1234567890abcdef"), "AKfy...cdef");
35923625
}
35933626

3627+
#[test]
3628+
fn parallel_relay_only_safe_for_idempotent_methods() {
3629+
// Locks down #743: parallel_relay must never fan-out non-idempotent
3630+
// methods because Apps Script can't be cancelled mid-request, so
3631+
// every concurrent attempt completes server-side and side-effects
3632+
// duplicate at the destination (comment posted twice, etc.).
3633+
for safe in ["GET", "HEAD", "OPTIONS", "get", "head", "options"] {
3634+
assert!(
3635+
is_method_safe_for_fanout(safe),
3636+
"{} should be safe for fan-out (idempotent per RFC 9110)",
3637+
safe,
3638+
);
3639+
}
3640+
for unsafe_m in ["POST", "PUT", "PATCH", "DELETE", "post", "put", "patch", "delete"] {
3641+
assert!(
3642+
!is_method_safe_for_fanout(unsafe_m),
3643+
"{} must NOT be safe for fan-out (non-idempotent — duplicate side-effects)",
3644+
unsafe_m,
3645+
);
3646+
}
3647+
// Unknown methods (CONNECT, TRACE, custom verbs) default to NOT
3648+
// safe — conservative call, matches the upstream `UrlFetchApp`
3649+
// lookup behavior.
3650+
for unknown in ["CONNECT", "TRACE", "PROPFIND", ""] {
3651+
assert!(
3652+
!is_method_safe_for_fanout(unknown),
3653+
"{} must default to NOT safe for fan-out when unrecognised",
3654+
unknown,
3655+
);
3656+
}
3657+
}
3658+
35943659
#[test]
35953660
fn parse_relay_array_set_cookie() {
35963661
let body = r#"{"s":200,"h":{"Set-Cookie":["a=1","b=2"]},"b":""}"#;

0 commit comments

Comments
 (0)