Skip to content

batch: parse_request_url never strips the query string, so every conditional entry is dispatched against a resource type containing the query #503

Description

@aacruzgon

Summary

parse_request_url splits a batch entry's request.url on / and nothing else. It never strips a query string, so every FHIR conditional interaction in a batch Bundle is parsed as a resource type that contains the query.

crates/rest/src/handlers/batch.rs:760:

fn parse_request_url(url: &str) -> Result<(String, String), String> {
    let parts: Vec<&str> = url.trim_start_matches('/').split('/').collect();

    match parts.len() {
        0 => Err("Empty URL".to_string()),
        1 => Ok((parts[0].to_string(), String::new())),
        2 => Ok((parts[0].to_string(), parts[1].to_string())),
        _ => Ok((parts[0].to_string(), parts[1].to_string())),
    }
}

For request.url == "Patient?identifier=http://example.org|12345" this returns:

  • resource_type = "Patient?identifier=http://example.org|12345"
  • id = ""

...except the | and // also interact with the / split, so the exact garbage depends on the query. Either way, no caller ever sees "Patient".

What that value is then used for

Both, in order, at crates/rest/src/handlers/batch.rs:404:

  1. The scope check (:420) — SmartScopePolicy::check(principal, &resource_type, operation) is asked about a resource type that does not exist. For a principal holding type-scoped grants (system/Patient.c) this fails closed with a per-entry 403, so it is not a privilege escalation. For a principal holding a wildcard grant (system/*.crud) the check passes and execution continues.

  2. The storage call (:465, :504, :540) — create / create_or_update / delete are invoked with the query-bearing string as the resource type.

Impact

Conditional create, conditional update and conditional delete are all unusable in a batch Bundle, and they fail in a way that does not say so:

  • Conditional update (PUT Patient?identifier=x) — parsed with an empty id, so create_or_update is called with id = "" rather than resolving the match.
  • Conditional delete (DELETE Patient?identifier=x) — same.
  • Conditional create (POST Patient + ifNoneExist) — separately unimplemented; process_batch_entry never reads ifNoneExist. parse_bundle_entry (:874) is its only reader and its only caller is process_transaction.

The transaction path is unaffected: it passes url through to the backend intact (:290), and the backends do their own conditional resolution.

I have not traced what each backend does when handed a resource type containing ? — whether it rejects it, or writes a row under that literal type. Worth establishing before deciding whether the fix is "parse the query and resolve the condition" or "reject conditional URLs on the batch path with a 400 until they are implemented".

Where it came from

Found while grounding #501. Pre-existing; unrelated to the concurrency work.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions