Skip to content

Url encode #1290 fix#1565

Open
pratt4 wants to merge 2 commits intoohs-foundation:masterfrom
pratt4:url-encode-1290-fix
Open

Url encode #1290 fix#1565
pratt4 wants to merge 2 commits intoohs-foundation:masterfrom
pratt4:url-encode-1290-fix

Conversation

@pratt4
Copy link
Contributor

@pratt4 pratt4 commented Feb 2, 2026

Fix paging URL encoding searchByUrl (Fixes #1290)

Context

Paging can break when Bundle.link[next].url contains reserved characters in query values (e.g. quotes).
FhirSearchUtil.searchByUrl(...) was passing the URL directly into HAPI client search, so decoded values could
produce invalid requests.

Root Cause

searchByUrl(...) forwarded the input URL as-is:

  • no query normalization
  • no RFC3986 escaping of query values before client.search().byUrl(...)

What changed

pipelines/batch/src/main/java/com/google/fhir/analytics/FhirSearchUtil.java

  • searchByUrl(...) now calls ensureQueryParamsArePercentEncoded(...) before byUrl(...).
  • Added ensureQueryParamsArePercentEncoded(String url):
    • splits into base + query + optional fragment
    • preserves separators (?, &, =) and fragment (#...)
    • preserves flag params (foo) and empty values (foo=)
    • re-encodes query values via HAPI UrlUtil.escapeUrlParam(...)
  • Added internal normalization so already-encoded values are not double-encoded.

Tests

pipelines/batch/src/test/java/com/google/fhir/analytics/FhirSearchUtilTest.java

  • Renamed/updated behavior test:
    • testHapiPreservesPercentEncodedUrlInBundle (expects %22 to stay %22)
  • Existing bug-focused test kept:
    • testSearchByUrlReEncodesQueryParams
  • Added:
    • testEnsureQueryParamsArePercentEncodedPreservesQueryStructure
    • testSearchByUrlDoesNotDoubleEncodeAlreadyEncodedValue

Result

searchByUrl(...) is now safe for both:

  • decoded query values (re-encoded before request), and
  • already percent-encoded query values (no double-encoding).

@pratt4 pratt4 requested a review from bashir2 February 2, 2026 16:21
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.79%. Comparing base (8434457) to head (86b2b93).

Files with missing lines Patch % Lines
...java/com/google/fhir/analytics/FhirSearchUtil.java 81.25% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1565      +/-   ##
============================================
+ Coverage     46.61%   46.79%   +0.17%     
- Complexity      678      685       +7     
============================================
  Files            90       90              
  Lines          5886     5915      +29     
  Branches        834      841       +7     
============================================
+ Hits           2744     2768      +24     
- Misses         2827     2830       +3     
- Partials        315      317       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When paging through search Bundles, next URLs are not properly escaped / encoded

2 participants