Fix: GOG chunk URL broken when CDN token is in query string

[unbelievableflavour]

Description

A fix applied in another project, but the fix applies to us as well. Tested and still works.

Recording

The412Banner/BannerHub@4f3c515

Checklist

• If I have access to #code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.
• I have attached a recording of the change.
• I have read and agree to the contribution guidelines in CONTRIBUTING.md.

---

Summary by cubic

Fix GOG chunk and Gen1 main.bin URLs when CDN tokens are in the query string. We now insert paths before the query to preserve Akamai __token__ params and avoid 403s.

• Bug Fixes
  • Insert chunk path before query string in GOGManifestParser for both global and per-product maps.
  • Add buildGen1MainBinUrl in GOGDownloadManager to place /main.bin before query; trims input and throws if URL is missing.
  • Delegate chunk URL map construction to GOGManifestParser and add tests for tokenized URLs and main.bin edge cases.

^{Written for commit 34a8e96. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes

  • Fixed download URL construction to insert file paths before existing query strings and normalize slashes, ensuring authentication tokens and query parameters are preserved.

• Tests

  • Added tests covering query-parameter handling and URL path normalization to prevent regressions in download operations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3c8b17b8-8924-4e99-9084-aad5bd30e596

📥 Commits

Reviewing files that changed from the base of the PR and between 7c7a211 and 34a8e96.

📒 Files selected for processing (4)

• app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
• app/src/main/java/app/gamenative/service/gog/api/GOGManifestParser.kt
• app/src/test/java/app/gamenative/service/gog/GOGDownloadManagerTest.kt
• app/src/test/java/app/gamenative/service/gog/api/GOGManifestParserTest.kt

✅ Files skipped from review due to trivial changes (1)

• app/src/test/java/app/gamenative/service/gog/api/GOGManifestParserTest.kt

🚧 Files skipped from review as they are similar to previous changes (2)

• app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
• app/src/test/java/app/gamenative/service/gog/GOGDownloadManagerTest.kt

---

📝 Walkthrough

Walkthrough

Refactors GOG URL construction: adds buildGen1MainBinUrl() in GOGDownloadManager, removes its local chunk-mapping implementation and delegates chunk URL assembly to GOGManifestParser which gains query-aware helpers to insert chunk paths before any existing query string.

Changes

Cohort / File(s)	Summary
Gen1 URL helper app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt	Added internal fun buildGen1MainBinUrl(baseUrl: String?) to normalize base URL and produce /main.bin while preserving query suffix; removed private buildChunkUrlMap(...) and call site now uses parser-provided mapping.
Chunk URL composition app/src/main/java/app/gamenative/service/gog/api/GOGManifestParser.kt	Removed duplicated inline chunk-path logic; added buildChunkUrl(baseCdnUrl, chunkMd5) and appendPathBeforeQuery(...) and updated buildChunkUrlMap(...) / buildChunkUrlMapWithProducts(...) to insert chunk paths before existing ? tokens and normalize slashes.
Tests app/src/test/java/app/gamenative/service/gog/GOGDownloadManagerTest.kt, app/src/test/java/app/gamenative/service/gog/api/GOGManifestParserTest.kt	Added tests for buildGen1MainBinUrl(...) (including query-preservation and null/blank validation) and tests asserting chunk-path insertion before existing query tokens in parser mappings.

Sequence Diagram(s)

(omitted — changes are refactor/utility-focused and do not introduce a new multi-component runtime control flow requiring visualization)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Gamefix: Added correct redist launch dep for Moonlighter & En Garde #638 — Modifies GOG downloader chunk/download behavior; closely related to URL/chunk mapping changes in this PR.

Suggested reviewers

• phobos665

Poem

🐰
I hopped through slashes, tamed the query's tail,
Placed main.bin snug where tokens pale.
Chunks now march before the ? gate,
Neat paths and tests — a tidy state.
🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically describes the main bug fix: GOG chunk URLs being broken when CDN tokens appear in query strings.
Description check	✅ Passed	The description includes the required sections (Description, Recording, Checklist) but the Recording field links to an external commit rather than attaching an actual recording as specified in the template.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

No issues found across 4 files

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 745-755: The function buildGen1MainBinUrl currently returns a
relative "/main.bin" when passed null/empty which later gets fed into
Request.Builder().url() and triggers an OkHttp IllegalArgumentException; change
buildGen1MainBinUrl to fail fast by requiring a non-null, non-blank baseUrl (use
require/baseUrl?.trim().isNotEmpty()) and throw a clear IllegalArgumentException
with an explanatory message if missing, so callers (the code that calls
buildGen1MainBinUrl before Request.Builder().url()) receive an immediate,
descriptive failure instead of a deferred runtime error.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cca4296f-ae7e-4768-b9f9-acf79aa0e5ba

📥 Commits

Reviewing files that changed from the base of the PR and between a29bb89 and 7c7a211.

📒 Files selected for processing (4)

• app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
• app/src/main/java/app/gamenative/service/gog/api/GOGManifestParser.kt
• app/src/test/java/app/gamenative/service/gog/GOGDownloadManagerTest.kt
• app/src/test/java/app/gamenative/service/gog/api/GOGManifestParserTest.kt