Skip to content

Build download pagination URLs with encoded query parameters - #84

Closed
sylvesterkaczmarek wants to merge 1 commit into
nasa:mainfrom
sylvesterkaczmarek:fix/pagination-query-parameters
Closed

sylvesterkaczmarek wants to merge 1 commit into
nasa:mainfrom
sylvesterkaczmarek:fix/pagination-query-parameters

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Overview

Fixes #83. No EDSC ticket has been assigned to this external contribution.

What is the feature?

Correct pagination requests when a getLinks endpoint has no query string or has a fragment, and preserve cursor tokens containing reserved characters. Direct string concatenation previously changed the request path or interpreted cursor contents as query/fragment syntax.

What is the solution?

Construct each request with URL and URLSearchParams, replace the pagination-owned pageNum/cursor fields, and retain other query values. Return to numbered pages when a response omits its cursor, as before. Trust checking, authorization, response validation, link insertion, initialization and error handling are unchanged. Update the client integration documentation and patch version to 1.0.10. The lockfile changes only the application's version, not dependency resolutions.

Testing

Added 15 Jest cases using the actual fetchLinks implementation and intercepted Electron network calls. They cover queryless/existing-query/fragment URLs, reserved and Unicode cursor values, repeated static parameters, stale pagination fields, fallback page numbers, stored links, initialization, authorization and request failures.

  • New tests against unchanged source: 9 failed, 6 passed.
  • Unchanged repository suite: 672 passed, 1 skipped across 102 suites.
  • npm test -- --runInBand --coverage=false: 687 passed, 1 skipped across 103 suites.
  • npm run lint: passed.
  • npm run build: passed for renderer, Electron main and preload bundles.
  • git diff --check: passed.

Tested on macOS with Node 26.8.1, based on main at 35d6269945e0cceaf4f7f24af633cdfbfd614a55. Dependencies were installed using npm ci --ignore-scripts; the version-only bump used npm version patch --no-git-tag-version --ignore-scripts. Tests retain the repository's Electron mocks. The live desktop application, packaged installers, native database binary, authentication flows, remote downloads and other operating systems were not exercised. No trusted-source, runtime-dependency or workflow changes are included. This does not overlap PR #80.

Checklist

  • Added regression tests and checked them against unchanged source.
  • New and existing runnable unit tests pass locally.
  • Reviewed the diff and documented the query/cursor contract.
  • Added a comment explaining query encoding.
  • Bumped application version and synchronized lockfile version fields; dependency entries are unchanged.

@macrouch

Copy link
Copy Markdown
Member

Are you representing a DAAC that wants to implement Earthdata Download, and this is an issue for your implementation?

Yes using URL and URLSearchParams is a better way to accomplish what we are doing, but this isn't a real issue right now. We aren't going to create a new release and push out to users unless there is a real need to do so.

@macrouch macrouch closed this Sep 28, 2026
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.

Pagination URLs mis-handle queryless endpoints and reserved characters in cursors

2 participants