SafeFile: demote oversized-string assertion to debug log (#246) - #617
Merged
mrjimenez merged 1 commit intoMay 15, 2026
Merged
Conversation
…ct#246) CFileDataIO::WriteStringCore wxFAIL_MSGs when asked to serialise a string larger than 0xFFFF bytes with a uint16 length prefix. The intent (per the surrounding comment) is to flag peer-supplied data that expands past 16 bits after ISO8859-1 -> UTF-8 conversion -- exactly the "external data should not trigger an assertion" pattern that PR amule-project#613 addressed for OP_IDCHANGE. The immediately-following code already does the right thing: it truncates real_length to 0xFFFF and adjusts sLength accordingly, so release builds silently absorb the oversized input. Debug builds, however, abort on what is harmless input -- crashing amuled on every periodic save where any single Kad contact field has been poisoned (see amule-project#246, drzraf 2021 and fekir 2024). Replace wxFAIL_MSG with AddDebugLogLineN(logCFile, ...) that records the original byte count for diagnosis without aborting. The truncation path is unchanged; on-disk format is unchanged. Same shape as PR amule-project#613 / issue amule-project#610. Reported by drzraf and fekir.
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jul 29, 2026
Closes amule-project#617. Downloads: a "Total queue size: X" field, right-aligned in the File sources row, summing the files currently visible (selected category + text filter). Kept live the same way as the file count -- reset in bulk by RebuildVisibleList(), adjusted by GetFileSize() as ShowFile() adds or removes a row -- so there is no per-tick cost. Shared: the combined size of the visible shared files, shown as "Total size of Shared Files: X" (reusing the existing string) in the statistics box, in place of the "Percent of total files" label -- which was misleading, since the gauges beside it show a session/all-time ratio per selected file, not a percentage of files. The session/all-time gauges stay. Both auto-scale units via CastItoXBytes and work in amuleGUI too: file sizes are already EC-streamed, so no core/protocol change is needed. The "Total queue size: %s" string is new; the shared label reuses an existing one. Catalogs regenerated.
ngosang
pushed a commit
to ngosang/amule
that referenced
this pull request
Jul 29, 2026
…project#689) Closes amule-project#617. Downloads: a "Total queue size: X" field, right-aligned in the File sources row, summing the files currently visible (selected category + text filter). Kept live the same way as the file count -- reset in bulk by RebuildVisibleList(), adjusted by GetFileSize() as ShowFile() adds or removes a row -- so there is no per-tick cost. Shared: the combined size of the visible shared files, shown as "Total size of Shared Files: X" (reusing the existing string) in the statistics box, in place of the "Percent of total files" label -- which was misleading, since the gauges beside it show a session/all-time ratio per selected file, not a percentage of files. The session/all-time gauges stay. Both auto-scale units via CastItoXBytes and work in amuleGUI too: file sizes are already EC-streamed, so no core/protocol change is needed. The "Total queue size: %s" string is new; the shared label reuses an existing one. Catalogs regenerated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CFileDataIO::WriteStringCorewxFAIL_MSGs when asked to serialise a string larger than0xFFFFbytes with auint16length prefix. Per the surrounding code's own comment, the trigger is peer-supplied data that expands past 16 bits after ISO8859-1 → UTF-8 conversion — exactly the "external data should not trigger an assertion" pattern that PR #613 addressed forOP_IDCHANGE. Crashes debug builds for the two reporters in #246 (drzraf 2021 writing Kad contacts; fekir 2024 startup/shutdown event loop).Why it's safe to demote
The immediately-following code already truncates the value and adjusts the BOM-adjusted
sLengthbefore writing — so release builds silently absorb the oversized input without on-disk corruption:The
wxFAIL_MSGtherefore guards no behaviour — release builds run the truncation path identically without it.Change
Replace
wxFAIL_MSGwithAddDebugLogLineN(logCFile, ...)that records the original byte count for diagnosis. The truncation path is unchanged; on-disk format is unchanged. Same shape as #613.Closes the assertion-symptom side of #246. The unrelated
epoll_ctlsymptom from the original 2021 report is likely incidentally fixed by86c4769d8(libcurl-basedHTTPDownloadreplacing the old wxFDIODispatcher path) but needs a fresh log to confirm; tracked separately.