Skip to content

feat: download .wpress backups from cloud storage URLs#52

Open
dugyen wants to merge 3 commits into
masterfrom
feat/cloud-storage-open
Open

feat: download .wpress backups from cloud storage URLs#52
dugyen wants to merge 3 commits into
masterfrom
feat/cloud-storage-open

Conversation

@dugyen

@dugyen dugyen commented May 19, 2026

Copy link
Copy Markdown

Summary

  • CloudDownloader (src/clouddownloader.{h,cpp}): downloads .wpress files from remote HTTPS URLs to a temp file via QNetworkAccessManager. Normalizes share links from Google Drive, Dropbox, OneDrive, Box, and pCloud into direct download URLs. Mega (mega.nz) uses a two-step flow: parse the AES-128-CTR key from the URL fragment (never sent to any server), call the Mega public API for a CDN URL, then decrypt the encrypted stream on-the-fly with OpenSSL EVP_aes_128_ctr().
  • Drop overlay (src/dropoverlay.{h,cpp}): accepts remote URLs in addition to local .wpress files. Two-path drag handling — text/uri-list primary, text/plain fallback — so cloud web-UI drags work and Mega #KEY fragments are preserved (text/uri-list strips fragments per RFC 2483).
  • URL dialog (src/urlopendialog.{h,cpp}): File → Open from URL… (⌘⇧O / Ctrl+Shift+O) lets the user paste any cloud share link or direct URL.
  • MainWindow wiring: urlDroppedopenBackupFromUrl, download progress bar with indeterminate mode when Content-Length is unknown, re-entrancy guard, temp file cleanup on clear/re-download.
  • 24 new tests in tst_clouddownloader.cpp covering isRemoteUrl, normalizeUrl (per provider), and parseMegaUrl (key derivation, legacy format, edge cases). All 121 tests pass.

Supported providers

Provider Normalization
Google Drive /file/d/{ID}/view and /open?id={ID}/uc?export=download&id={ID}&confirm=t
Dropbox dl=0dl=1
OneDrive / 1drv.ms Appends download=1
Box /s/{code}/shared/static/{code}
pCloud publink/show?code=Xpublink/code?code=X&forcedownload=1
Mega Two-step: API call for CDN URL + AES-128-CTR decryption (key from URL fragment)
S3 / GCS / Azure / DO Spaces Pass-through (pre-signed or public URLs work as-is)

Security notes

  • The Mega decryption key lives in the URL fragment and is never transmitted to any server.
  • No telemetry, no background connections. QNetworkAccessManager is only used when the user explicitly provides a URL.
  • Downloaded files land in QTemporaryFile in the system temp dir and are cleaned up on clear or re-download.
  • FTP is explicitly rejected — Qt 6 removed FTP from QNetworkAccessManager.

Test plan

  • Drag a local .wpress file — still works as before
  • Drag a Google Drive share link from Chrome → overlay accepts, file downloads
  • Drag a Dropbox share link → downloads with dl=1
  • Drag a Mega link → two-step download + AES decryption produces valid .wpress
  • File → Open from URL → paste a OneDrive link → downloads
  • Paste an invalid URL → OK button stays disabled
  • Cancel mid-download → temp file is cleaned up
  • Drop a second URL while downloading → ignored (re-entrancy guard)
  • ctest --output-on-failure — all 121 tests pass (97 existing + 24 new)
  • clang-format --dry-run --Werror — no violations

🤖 Generated with Claude Code

@github-actions github-actions Bot added build Build system/project files ui User interface docs Documentation labels May 19, 2026
@github-actions

github-actions Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Build Artifacts

Platform Download
Linux (x86_64) Traktor-linux-x86_64
macOS (Apple Silicon) Traktor-macOS
Windows (x64) Traktor-windows-x64

Built from f326e8e. Artifacts expire after 90 days.

@developeritsme

developeritsme commented May 19, 2026

Copy link
Copy Markdown
Contributor

Code Review

Thanks for the PR - the feature is well-organized and the signal/slot wiring is clean. However, all 4 substantive CI checks are failing (Linux/macOS/Windows builds + Code Style), and there are correctness issues that need to be addressed before this can merge. I also checked the branch out and reproduced the failures locally.

Blockers

1. urlopendialog.cpp does not compile - smart quotes used as string delimiters

Lines 40-43 (the hintLabel "Tip:" block) use Unicode curly quotes (U+201C / U+201D) instead of ASCII " to delimit the string passed to tr(). This fails on every platform:

src/urlopendialog.cpp:40:12: error: unexpected character <U+201C>

Every other string in the file uses straight quotes correctly. Fix is mechanical: replace the curly-quote delimiters with " and escape the inner quotes.

2. clang-format violations (Code Style check)

The project's formatter was not run. clang-format --dry-run --Werror flags:

  • src/clouddownloader.cpp - lines 15-17, 32-33, 247, 295-297, 335-337, 359, 399, 401, 442
  • src/dropoverlay.cpp - lines 45-46
  • src/mainwindow.h - line 55
  • src/urlopendialog.cpp - lines 23-28

Fix per CLAUDE.md:

find src tests \( -name '*.cpp' -o -name '*.h' \) ! -name 'moc_*' | xargs clang-format -i

3. PR description contradicts the implementation

The description says "Note on Mega: ... not supported in this PR." but the branch HEAD (feat: add Mega (mega.nz) cloud storage support) fully implements it - parseMegaUrl, downloadMega, AES-128-CTR decryption. CLAUDE.md and the dialog also advertise Mega. Please update the description so reviewers can evaluate the (now in-scope) Mega code and its security claims.

Correctness

4. Mega AES key is likely used without un-mangling. Mega file links embed a 256-bit key; the real 128-bit AES key is the XOR of the two 16-byte halves (key[0..15] ^ key[16..31]), with bytes 16-23 as the nonce. The code uses keyBytes.left(16) directly as the AES key, which should decrypt to garbage. Please verify against the Mega protocol - Mega support should not ship without a round-trip test using a known key/ciphertext vector.

5. Mega multi-part (g as array) uses only the first segment and assumes redirects fill the rest. If that branch is taken, the temp file will be silently truncated/corrupt with no error. Either reassemble all parts or emit failed() explicitly.

6. FTP is claimed but not supported in Qt 6. isRemoteUrl() accepts ftp/ftps and the description/dialog advertise FTP "via Qt's built-in FTP handler", but the FTP backend was removed from QNetworkAccessManager in Qt 6 (the project targets Qt 6.8). ftp:// URLs will error out. Drop the FTP claim/scheme, or back it with a real implementation.

Bugs

7. Temp files leak. QTemporaryFile is created with setAutoRemove(false), and disk cleanup happens only via m_pendingTempFile, which is set only on onDownloadFinished() (success). As a result:

  • Failed downloads leave the temp file on disk (not removed in onFinished()/onDownloadFailed()).
  • abort() doesn't remove the temp file.
  • Repeated downloads: startCdnDownload() does delete m_tempFile, which frees the object but leaves the file on disk (autoRemove is off) - each download leaks the previous one.

Centralize cleanup: remove the temp file in abort(), in the failure path of onFinished(), and before creating a new one.

8. EVP_DecryptUpdate return value ignored in onReadyRead() - a decrypt failure on a chunk silently writes garbage. Check the return and emit failed().

9. No content validation of the downloaded file. Google Drive's confirm=t trick is fragile - for larger files Drive often returns an HTML interstitial instead of the file. The bytes are handed straight to openBackupFile() with no content-type or magic-byte check, so the user sees a confusing parse error instead of "this link isn't a direct download."

Test coverage

No tests added. isRemoteUrl(), normalizeUrl(), extractGoogleDriveId(), and parseMegaUrl() are pure/static functions and trivial to unit-test - exactly the logic that breaks silently (see items 4-6). The project has 97 tests and CLAUDE.md emphasizes coverage; please add tst_ cases for per-provider URL normalization and Mega URL/key parsing (new + legacy formats + invalid input).

Code quality

  • Dead code in parseMegaUrl() - a six-line comment block explains an abandoned design and ends with (void)fileId;. fileId is parsed, discarded, then re-parsed in downloadMega(). Remove the parsing and the comment block.
  • Indeterminate progress - progress(-1) is rendered as setValue(0), which looks like a stuck bar. Use setRange(0,0) for a true busy indicator.
  • Re-entrancy - if a second URL is dropped mid-download, openBackupFromUrl() updates the UI to "Downloading..." but CloudDownloader::download() returns early silently, so UI and downloader state diverge. Guard at the MainWindow level too.

Security

The user-initiated-only network model is reasonable, and the CLAUDE.md security note was honestly updated. Two points:

  • This PR fundamentally changes the app's "zero network requests, fully offline" posture. That's a product decision worth an explicit sign-off, not just a doc edit.
  • The "key never leaves the client" claim for Mega is accurate for the decryption key, but the file ID is necessarily sent to g.api.mega.co.nz - state that precisely.

Recommendation

Request changes. Fix the build (item 1), formatting (item 2), and description (item 3) blockers, plus the temp-file leaks (item 7) and tests (item 9). For Mega: given the unverified key handling (item 4) and multi-part gap (item 5), I'd suggest splitting Mega into a separate follow-up PR with proper test vectors, and landing the simpler, well-understood provider normalization (Drive/Dropbox/OneDrive/Box/pCloud/S3) first - that part looks solid.

@github-actions github-actions Bot added the test Test suite changes label May 19, 2026
Add CloudDownloader (src/clouddownloader.{h,cpp}) which downloads a
.wpress file from a remote HTTPS URL to a temp file. Normalizes share
links from Google Drive, Dropbox, OneDrive, Box, and pCloud into direct
download URLs. Mega (mega.nz) is handled via a two-step flow: parse the
AES-128-CTR key from the URL fragment (never sent to any server), call
the Mega public API for a CDN URL, then decrypt the stream on-the-fly
with OpenSSL EVP_aes_128_ctr().

Drop overlay (dropoverlay.{h,cpp}): accept remote URLs in addition to
local .wpress files. Two-path drag handling — text/uri-list primary,
text/plain fallback — so cloud web-UI drags work and Mega #KEY fragments
are preserved (text/uri-list strips fragments per RFC 2483).

URL dialog (urlopendialog.{h,cpp}): File → Open from URL (Ctrl+Shift+O)
lets the user paste any cloud share link or direct URL.

MainWindow wiring: urlDropped signal → openBackupFromUrl slot, download
progress bar with indeterminate mode (setRange(0,0)) when Content-Length
is unknown, re-entrancy guard (m_downloading flag), temp file cleanup on
clear/re-download.

Tests: 24 new test cases in tst_clouddownloader.cpp covering isRemoteUrl,
normalizeUrl (per provider), and parseMegaUrl (key derivation, legacy
format, edge cases). All 121 tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@dugyen dugyen force-pushed the feat/cloud-storage-open branch from 6784c3a to 8c3119c Compare May 19, 2026 15:36
@dugyen dugyen changed the title feat: open .wpress backups directly from cloud storage URLs feat: download .wpress backups from cloud storage URLs May 19, 2026
The .ui file already defines a File menu (menu_File). The previous code
created a second File menu via menuBar()->addMenu("&File"), which on
macOS gets silently merged/hidden — making "Open from URL…" invisible.

Fix: insert the action into ui->menu_File (after "Open backup", before
"Clear file") instead of creating a duplicate menu.

Also update the drop zone hint text from "Drop .wpress file here" to
"Drop .wpress file or cloud link here" to hint at URL drop support.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added the ci CI/CD workflows label May 20, 2026
@developeritsme

Copy link
Copy Markdown
Contributor

Could you split 7959b59 (the GitHub Actions / Node.js 24 bump) into its own ci: PR? It's unrelated to the cloud-storage feature, and actions/upload-artifact v4 → v7 in particular is a big enough jump that it should be reviewable and revertable on its own.

@dugyen dugyen force-pushed the feat/cloud-storage-open branch from f9a3e18 to b38af16 Compare May 20, 2026 13:59
@developeritsme developeritsme removed the ci CI/CD workflows label May 20, 2026
Add an "Open URL..." button in the bottom button row, between "Open
backup..." and "Clear", so the cloud-storage feature is immediately
discoverable without needing the File menu or keyboard shortcut.

The button is wired to openFromUrl() via the .ui file and is disabled
during downloads (same as the other buttons).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Build system/project files docs Documentation test Test suite changes ui User interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants