This PR adds local Wi‑Fi backup transfer between 3DS consoles.#528
Conversation
- fix const-correctness in Title API used by transfer (`id()` and `shortDescription()`) - fix `Server::isRunning()` by using an explicit atomic runtime state - replace deprecated `std::wstring_convert` usage with `StringUtils::UTF16toUTF8` - mark pending ZIP transfer helpers/constants as `[[maybe_unused]]` to avoid noise - make `ccache` optional in 3DS Makefile to remove repeated "which: no ccache" output - set `GIT_REV` fallback to `unknown` when building outside a git repository
Make file sending compatible with savedata containing multiple internal files. Also translated some remaining parts that were still in Spanish into English. UI update after receiving data is still pending—currently, data only displays after closing and reopening. Additionally, some interface elements still need to be updated to improve clarity and consistency.
- add receiver completion and pending-refresh state, and trigger a full titles refresh from `MainScreen` - improve transfer overlays (clearer send/receive prompt, shortcuts, and post-receive confirmation view) - add fallback mapping by title name when title ID is missing/mismatched, with clearer user notices - include folder entries in ZIP packaging and return a specific error for empty backups - harden ZIP read/write and HTTP response handling to report corruption and receiver-side errors - normalize line endings in GitHub issue templates
|
Thanks for the contribution. I'll review as soon as I can |
The PIN is the only authentication for the upload endpoint, but the /transfer/info handler returned it in plaintext to any unauthenticated caller on the LAN, letting anyone read it and then POST to /transfer/upload. The sender never consumed this field anyway. Remove the token from the info response; the PIN is now only shown on the receiver's screen and must be entered manually on the sender. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The server buffered the entire HTTP request in a std::string driven by an attacker-controlled Content-Length, and parseMultipart then copied the whole body and the file part again (~3x the file size resident at once). On the 3DS's small heap this could OOM/crash on large saves or via a crafted Content-Length. - Cap the buffered request at MAX_REQUEST_SIZE (32 MiB) and reply 413 when exceeded, also guarding against a client that never terminates headers. - parseMultipart now returns the file part as an [offset,length) range into the original buffer instead of copying it, so the payload exists only once in memory and is written straight to disk. Also drops a stray trailing-CRLF trim that could truncate binary payloads ending in 0x0D0A by two bytes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The server runs on a single thread and the accepted socket is blocking, so a client that connects and then stalls would block the receiver forever: stopReceiver() only unregisters handlers and never closes the socket, and no further connections could be accepted. Set SO_RCVTIMEO (15s) on each client socket so a stalled recv unblocks, the request is abandoned, and the server returns to accepting. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
g_transferMode (a std::string) and the g_transferBytes{Done,Total}
counters are written by the HTTP server thread while receiving and read
by the UI thread when drawing progress. Concurrent access to the
std::string is undefined behavior (and can crash), and 64-bit reads tear
on the 3DS's 32-bit core.
Route all access through mutex-protected helpers (transferSetMode/
transferGetMode/transferSetProgress/transferAddDone/transferGetProgress)
and protect the receiver notice/completed-name strings with their own
mutex behind setReceiverNotice/setReceiverCompletedName and locked
getters. UI readers now take a consistent snapshot under the lock.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Transfer start/stop now registers and unregisters HTTP handlers at runtime (main thread) while the network thread concurrently looks them up, racing on the std::map. Guard all access with a mutex; the request dispatcher copies the handler out under the lock and releases it before invoking the handler so a long upload doesn't block (un)registration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
BernardoGiordano
left a comment
BernardoGiordano
left a comment
There was a problem hiding this comment.
A few low-severity quality/robustness notes from the review (the higher-severity security and concurrency items are being addressed in separate commits).
| } | ||
|
|
||
| for (auto& entry : files) { | ||
| entry.crc = computeCrc(Archive::sdmc(), entry.absPath); |
There was a problem hiding this comment.
Perf (low): every file is read fully twice during packaging — once here in computeCrc and again below to stream the data into the ZIP. On the 3DS's slow SD this roughly doubles the packaging time for large backups. Consider computing the CRC incrementally during the single write pass (update the CRC over each buffer as you write it) and backfilling the local-header CRC, or use a data descriptor.
| input.close(); | ||
| return false; | ||
| } | ||
| (void)crc; |
There was a problem hiding this comment.
Robustness (low): the per-entry CRC is read from the ZIP and then discarded ((void)crc;), so a corrupted/truncated transfer is silently extracted as-is. Since you already have a CRC routine, consider verifying the extracted data against crc and failing the upload on mismatch.
| size_t headerEnd = requestData.find("\r\n\r\n"); | ||
| std::string headers = headerEnd == std::string::npos ? requestData : requestData.substr(0, headerEnd); | ||
| std::string token = headerValue(headers, "X-CP-Token"); | ||
| if (token != g_token) { |
There was a problem hiding this comment.
Minor: token != g_token is not a constant-time comparison. Low risk on a LAN with a short-lived PIN, but a constant-time compare is cheap insurance against timing-based PIN guessing.
| memset(&addr, 0, sizeof(addr)); | ||
| addr.sin_family = AF_INET; | ||
| addr.sin_port = htons(port); | ||
| addr.sin_addr.s_addr = inet_addr(ip.c_str()); |
There was a problem hiding this comment.
Robustness (low): inet_addr is deprecated and returns 0xFFFFFFFF (i.e. 255.255.255.255) on malformed input rather than signalling an error, so a typo'd IP silently produces a bogus destination. Prefer inet_pton(AF_INET, ip.c_str(), &addr.sin_addr) and surface an "Invalid IP" error when it returns != 1.
| std::string backupName = nameFromCell(cellIndex); | ||
| std::u16string backupPath = Archive::mode() == MODE_SAVE ? title.fullSavePath(cellIndex) : title.fullExtdataPath(cellIndex); | ||
|
|
||
| std::string ipPort = rawKeyboard("192.168.1.10:8000", "IP:PUERTO del receptor", 32); |
There was a problem hiding this comment.
Consistency (low): these keyboard hints are in Spanish ("IP:PUERTO del receptor" here and "PIN (4-6 digitos)" at line 809) while the rest of the UI is English. Worth aligning the language for the upstream project.
| if (pin.empty()) { | ||
| return; | ||
| } | ||
| bool pinOk = pin.size() >= 4 && pin.size() <= 6 && |
There was a problem hiding this comment.
Inconsistency (low): the sender accepts a 4–6 digit PIN, but the receiver always generates exactly 4 digits (startReceiver: 1000 + rand() % 9000). A 5–6 digit PIN entered here can therefore never match. Either generate a variable-length PIN on the receiver or restrict the sender to 4 digits.
| CFLAGS += $(INCLUDE) -DARM11 -D__3DS__ -D_GNU_SOURCE=1 | ||
|
|
||
| CXXFLAGS := $(CFLAGS) -fno-rtti -std=gnu++23 | ||
| CXXFLAGS := $(CFLAGS) -fno-rtti -std=gnu++23 |
There was a problem hiding this comment.
Nit: trailing whitespace introduced after -std=gnu++23.
|
I pushed fixes for the security/concurrency findings (PIN exposure via |
The top-screen transfer loader drew the mode label, percentage and byte counts as one centered line, so longer labels like "Preparing backup package" ran off-screen and didn't match the new modal loader. Split it into two centered lines (label/percent, then size) like the bottom modal. Also format the transferred/total size in MB across the top loader, the bottom progress modal, and the receiver overlay instead of raw bytes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Security & concurrency hardening for the Wi-Fi transfer feature
Resolve conflicts keeping the Wi-Fi transfer feature alongside upstream's recent work (system saves, asian fonts, utf8/utf16 refactor, multi-file savedata progress UI): - 3ds/include/main.hpp: keep the thread-safe transfer status helpers and add upstream's g_multiSelectCount/g_multiSelectTotal globals. - 3ds/source/MainScreen.cpp: keep the network-transfer progress modal (g_transferIsNetwork branch) and adopt upstream's improved local-copy modal with the multi-select overall progress bar. - 3ds/source/server.cpp: keep the atomic serverIsRunning concurrency fix. - common/common.cpp: take upstream's removal of UTF16toUTF8 (relocated to 3ds/source/util.cpp, rewritten without the deprecated codecvt). Builds clean for the 3ds target (Checkpoint.3dsx/.cia produced). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ordano#528 - transfer.cpp (writeZip): compute each file's CRC during the single streaming pass and backfill it into the local header, instead of a separate computeCrc pre-pass. Halves file reads when packaging large backups on the slow SD. - transfer.cpp (extractZip): verify the extracted data against the CRC stored in each ZIP entry and fail on mismatch, so a corrupted or truncated transfer is rejected instead of written out as-is. - transfer.cpp (handleUpload): compare the PIN token in constant time to avoid leaking it through comparison timing. - transfer.cpp (sendBackup): parse the destination IP with inet_pton and surface an 'Invalid IP address' error instead of inet_addr, which maps malformed input to 255.255.255.255. - MainScreen.cpp: translate the IP/PIN keyboard hints to English and require an exactly-4-digit PIN, matching the receiver which always generates 4 digits (a 5-6 digit PIN could never match). - Makefile: drop trailing whitespace after -std=gnu++23. Builds clean for the 3ds target. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the thorough review! I used Claude Code to help resolve the conflicts and work through your feedback, so apologies in advance if anything looks off — happy to adjust. I've merged the latest upstream/master to resolve the conflicts, and addressed all the inline comments:
|
2 participants
This PR adds local Wi-Fi backup transfer between 3DS consoles in Checkpoint.
What this adds
New Transfer flow in the 3DS UI (
Send/Receive).Receiver mode with IP / port / PIN token display.
Sender upload of the selected save/extdata backup over HTTP multipart.
Receiver endpoints for transfer negotiation and upload handling.
Support for both:
Post-transfer completion state, automatic title list refresh.