fix(fifa17): make an unhandled club defId= list visible instead of silent
A health sweep of all 51 host routes found no errors, but did find a gap against the documented club grammar: the client may send a comma-joined `defId=` list INSTEAD of the filter block, and `parse_club_query` handled nine parameters without it. Today such a request is answered with the whole filtered club rather than the requested definitions — silently. Deliberately NOT implementing the filter. That grammar is single-source (one decompile plus one live log line, and the log line carried no defId), so the reading of "definition id" is unconfirmed against any observed request. Narrowing on a wrong reading would turn "too many items" into "zero items", which is the worse failure and the harder one to diagnose. So the parameter is parsed and reported instead: the filter summary gains `defId=<n>` and the host logs a NOTICE naming the ids and saying plainly that the response was not narrowed. The first real occurrence is then impossible to miss, and the filter can be written against a captured request rather than a guess. Verified live: the notice fires and total stays 1966.
This commit is contained in:
@@ -69,6 +69,18 @@ pub struct Fifa17ClubQuery {
|
||||
pub start: Option<u32>,
|
||||
/// Pagination page size.
|
||||
pub count: Option<u32>,
|
||||
/// `defId=` — a comma-joined list of DEFINITION ids. The documented club
|
||||
/// grammar says the client sends EITHER the filter block above OR this list,
|
||||
/// never both.
|
||||
///
|
||||
/// CAPTURED BUT NOT APPLIED, deliberately. The grammar is single-source (one
|
||||
/// decompile plus one live log line, and that log line carried no `defId`),
|
||||
/// so the exact semantics — definition id as `resourceId`, presumably — are
|
||||
/// not confirmed against an observed request. Filtering on a wrong reading
|
||||
/// would turn "too many items" into "zero items", which is the worse failure.
|
||||
/// The host logs it instead, so the first real occurrence is visible and the
|
||||
/// filter can be written against evidence rather than a guess.
|
||||
pub def_ids: Vec<i64>,
|
||||
}
|
||||
|
||||
/// Minimal percent/`+` decoding, dependency-free. FIFA sends bare tokens and
|
||||
@@ -128,6 +140,14 @@ pub fn parse_club_query(query: &str) -> Fifa17ClubQuery {
|
||||
"sort" => out.sort = Some(v),
|
||||
"start" => out.start = v.parse().ok(),
|
||||
"count" => out.count = v.parse().ok(),
|
||||
// Comma-joined, digits only — the same reading the item-definition
|
||||
// routes already use for this parameter.
|
||||
"defId" => {
|
||||
out.def_ids = v
|
||||
.split(',')
|
||||
.filter_map(|d| d.trim().parse::<i64>().ok())
|
||||
.collect()
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
@@ -339,6 +359,7 @@ mod tests {
|
||||
sort: Some("desc".into()),
|
||||
start: Some(10),
|
||||
count: Some(11),
|
||||
def_ids: Vec::new(),
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -528,3 +549,25 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod def_id_tests {
|
||||
use super::*;
|
||||
|
||||
/// `defId=` is CAPTURED so the host can report it, and deliberately does NOT
|
||||
/// participate in the Core query — narrowing on an unconfirmed reading of the
|
||||
/// parameter would answer "zero items" where today we answer "too many".
|
||||
#[test]
|
||||
fn def_id_list_is_captured_but_never_narrows_the_core_query() {
|
||||
let q = parse_club_query("?year=2017&type=player&defId=20801,117617092,84044103");
|
||||
assert_eq!(q.def_ids, vec![20801, 117_617_092, 84_044_103]);
|
||||
assert_eq!(q.item_type.as_deref(), Some("player"));
|
||||
|
||||
// Non-numeric entries are dropped rather than poisoning the list.
|
||||
let q = parse_club_query("?defId=20801,,notanid,42");
|
||||
assert_eq!(q.def_ids, vec![20801, 42]);
|
||||
|
||||
// Absent means empty, never a phantom filter.
|
||||
assert!(parse_club_query("?type=player").def_ids.is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2206,6 +2206,22 @@ pub fn handle_club(query: &str, deps: &ClubDeps<'_>) -> (WireResponse, ClubLog)
|
||||
if !deps.hidden.is_empty() {
|
||||
filter.push_str(&format!(",hidden={}", deps.hidden.len()));
|
||||
}
|
||||
// The documented club grammar allows a comma-joined `defId=` list INSTEAD of
|
||||
// the filter block, and this host does not narrow on it. That grammar is
|
||||
// single-source and no observed request has ever carried one, so guessing the
|
||||
// semantics could turn "too many items" into "zero items". Make the first
|
||||
// real occurrence impossible to miss instead of silently answering wrong.
|
||||
if !raw.def_ids.is_empty() {
|
||||
filter.push_str(&format!(",defId={}", raw.def_ids.len()));
|
||||
eprintln!(
|
||||
"utas-host owner=RUST route=club NOTICE unhandled defId list ({} id(s): {:?}) \
|
||||
— the response is NOT narrowed to them. This is the first observation of a \
|
||||
parameter only ever seen in a decompile; capture the full request and \
|
||||
implement the filter against it.",
|
||||
raw.def_ids.len(),
|
||||
&raw.def_ids[..raw.def_ids.len().min(8)]
|
||||
);
|
||||
}
|
||||
|
||||
match deps.core.query_owned(&base.to_query_pairs()) {
|
||||
Ok(page) => {
|
||||
|
||||
Reference in New Issue
Block a user