Skip to content

feat: include AICH in generated magnet links, parse it back on load - #651

Merged
got3nks merged 2 commits into
amule-org:masterfrom
LSalami:add-aich-magnet-links
Jul 27, 2026
Merged

feat: include AICH in generated magnet links, parse it back on load#651
got3nks merged 2 commits into
amule-org:masterfrom
LSalami:add-aich-magnet-links

Conversation

@LSalami

@LSalami LSalami commented Jul 27, 2026

Copy link
Copy Markdown

Summary

Fixes #331 — two related asks:

  • Generated magnet links should include the AICH hash when available.
  • A magnet link carrying an AICH hash should have it parsed and used when converted to an ed2k link on download add.

Implementation

CreateMagnetLink() now adds an xt=urn:aich:<hash> field alongside the
existing ed2k/ed2khash ones whenever the file has a proper AICH hash set —
same source and guard CreateED2kLink() already uses for the ed2k link's
h= field.

CMagnetED2KConverter::GetED2KLink() — used both when a user pastes a
magnet: link (CDownloadQueue::AddLink) and by the ed2k CLI tool —
now looks for that urn:aich: field alongside the ed2k one and, when
present, embeds it as h=<aich>| in the ed2k link it builds. That string
then flows through the existing ed2k link parser (ED2KLink.cpp
already handles h=), so the AICH hash reaches the download with no
second parsing path to add or keep in sync — the second half of the
request falls out of the first for free.

Test plan

Added MagnetURITest (4 cases) since this is pure string logic with no
GUI/network dependency — more reliable than exercising it by hand through
the app:

  • GetLinkIncludesAichField — field round-trips into the generated magnet link

  • ConvertsAichUrnIntoEd2kHField — AICH urn converts into the ed2k h= field, correctly positioned before the closing /

  • NoAichUrnMeansNoHField — no AICH urn means no h= field (regression check)

  • AichUrnOrderBeforeEd2kUrnStillWorks — field order in the magnet doesn't matter

  • Verified both compile modes MagnetURI.cpp supports build cleanly: the main amule app (wxString) and the standalone ed2k CLI tool (USE_STD_STRING)

  • Not verified end-to-end through the GUI with a real peer serving an AICH-bearing file (impractical to simulate) — the conversion logic is unit-tested in isolation and the downstream parser is pre-existing/unchanged, so residual risk is low

…mule-org#331)

CreateMagnetLink() now adds an "xt=urn:aich:<hash>" field alongside the
existing ed2k/ed2khash ones whenever the file has a proper AICH hash
set -- same source and guard CreateED2kLink() already uses for the
ed2k link's "h=" field.

CMagnetED2KConverter::GetED2KLink() -- used both when a user pastes a
magnet: link (CDownloadQueue::AddLink) and by the "ed2k" CLI tool --
now looks for that urn:aich: field alongside the ed2k one and, when
present, embeds it as "h=<aich>|" in the ed2k link it builds. That
string then flows through the *existing* ed2k link parser
(ED2KLink.cpp already handles "h="), so the AICH hash reaches the
download with no second parsing path to add or keep in sync.

Added MagnetURITest (4 cases: field round-trips into the magnet link,
AICH urn converts into the ed2k h= field, no AICH urn means no h=
field, and field order in the magnet doesn't matter) since this is
pure string logic with no GUI/network dependency -- a much more
reliable check than exercising it by hand through the app. Verified
both compile modes MagnetURI.cpp supports: the main app (wxString)
and the standalone "ed2k" CLI tool (USE_STD_STRING).

@got3nks got3nks left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both directions covered, and reusing the existing h= parser on import is clean. One guard I'd like before merge:

The AICH token is passed through opaquely, but the ed2k parser throws on a bad master-hash (ED2KLink.cpp: throw wxString("Invalid master-hash")). So a magnet with a junk/truncated urn:aich: converts to …|h=<garbage>|/ and gets rejected wholesale — even though its ed2k hash is valid. Previously the AICH urn was just ignored, so it couldn't break the add.

Could you only embed h= when the token is well-formed (32 chars, base32 [A-Z2-7]), and drop it otherwise? A plain string check works in both build modes (no CAICHHash dependency), plus a MalformedAichUrnIsDropped test to lock it in. Keeps the invariant "valid ed2k magnet ⇒ always addable, junk AICH ignored."

Rest LGTM.

ED2KLink.cpp's ed2k-link parser throws on a bad master-hash, so a magnet
carrying a junk or truncated urn:aich: converted straight into a
"...|h=<garbage>|/" ed2k link that got rejected wholesale on import --
even though its ed2k hash was perfectly valid and previously (before
urn:aich: support) would have been ignored harmlessly.

Validate the AICH token as 32 chars of base32 [A-Z2-7] before embedding
it, and drop it silently otherwise, keeping the invariant that a valid
ed2k magnet is always addable regardless of what a malformed AICH
tagalong looks like. Plain char-range check, no CAICHHash dependency,
so it compiles the same in both build modes.

Addresses got3nks's review comment on amule-org#651.
@LSalami

LSalami commented Jul 27, 2026

Copy link
Copy Markdown
Author

Good catch. Fixed in 67d9366: GetED2KLink() now validates the AICH token (32 chars, base32 [A-Z2-7], plain char-range check — no CAICHHash dependency, so it's identical in both build modes) before embedding it into h=, and silently drops it otherwise. A junk/truncated urn:aich: now converts exactly as if it weren't there — the ed2k hash always goes through.

Added MalformedAichUrnIsDropped covering both failure shapes (too-short, and right-length-but-non-base32), plus the existing tests still cover the happy path. Ran the full MagnetURITest binary locally (all 5 cases pass) and also compiled+linked+ran a standalone harness exercising GetED2KLink() directly to double check the runtime behavior, not just that it compiles.

@got3nks got3nks left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guard's in and both failure shapes are covered by the new test — a malformed urn:aich: now drops cleanly and the valid ed2k hash always goes through. LGTM.

@got3nks
got3nks merged commit 9dc9009 into amule-org:master Jul 27, 2026
13 checks passed
@LSalami
LSalami deleted the add-aich-magnet-links branch July 27, 2026 20:22
LSalami added a commit to LSalami/amule that referenced this pull request Jul 27, 2026
Mechanical rebase to resolve the po/ catalog conflicts against master's
amule-org#648/amule-org#650/amule-org#651 (per got3nks's note on amule-org#643) -- no source changes here,
scripts/update-po.sh output only, so the diff is just the tree back in
sync with the current strings.
LSalami added a commit to LSalami/amule that referenced this pull request Jul 28, 2026
Mechanical rebase to resolve the po/ conflicts against master's
amule-org#643/amule-org#646/amule-org#648/amule-org#650/amule-org#651 -- no source changes here. Confirms got3nks's
prediction on the Alt+<letter>-outside-_() fix: the only genuinely new
msgid in the diff is "Navigate" (the new macOS menu itself); every
other hunk is line-number-comment churn from the rebase, not orphaned
or duplicated translations.
LSalami added a commit to LSalami/amule that referenced this pull request Jul 28, 2026
Mechanical rebase to resolve the po/ catalog conflicts against master's
amule-org#648/amule-org#650/amule-org#651 (per got3nks's note on amule-org#643) -- no source changes here,
scripts/update-po.sh output only, so the diff is just the tree back in
sync with the current strings.
got3nks pushed a commit that referenced this pull request Jul 28, 2026
* feat(search): persist search history across restarts (#641)

The search field was a plain text control with no memory of past
searches. Swap it for a wxComboBox (a wxTextEntry, like wxTextCtrl,
so the existing GetValue()/Clear() call sites keep working via that
shared base) and persist submitted terms to wxConfig, most-recent
first, capped at 20 entries and deduplicated case-insensitively.

Right-clicking the field opens a small menu ("Remember search
history" checkbox + "Clear search history") so history can be paused
or wiped without a dedicated Preferences page.

i18n: ran scripts/update-po.sh to register the two new menu strings.

* style(search): separate the destructive Clear item from the toggle above

A separator between the checkbox and "Clear search history" keeps the
one-shot destructive action visually distinct from the persistent
on/off state, instead of reading as a second toggle in the same group.

* style(search): use nullptr instead of NULL for the new combo box

Fixes the modernize-use-nullptr clang-tidy finding on the changed
line; matches the nullptr convention already used a couple of
wxComboBox constructions below in the same file.

* feat(search): rework query history per #643 review — file, prefs toggle, edit menu

Addresses all four blocking points from got3nks's review, plus both
optional suggestions:

1. Edit context menu restored. Overriding the field's context menu no
   longer drops Cut/Copy/Paste/Select All -- same custom-Paste-ID idiom
   as CMuleTextCtrl::OnRightDown (wxMenu over-permits wxID_PASTE, so it
   gets a manual clipboard-content check), plus a separator, plus the
   history's own Clear action.

2. Persisted to a dedicated searchhistory.dat (via CTextFile, one term
   per line) instead of amule.conf -- mirrors eMule's AC_SearchStrings.dat.
   Query terms no longer bloat the main config or travel with config
   backups / the --amule-config-file push to amuleweb/amuleapi.

3. "Remember search history" moved into Preferences > General as an
   ordinary Cfg_Bool checkbox (IDC_SEARCHHISTORYENABLED), same
   NewCfgItem wiring as every other GUI-behavior toggle on that page.
   It's a client-side-only setting, so the one checkbox works unchanged
   in both amule and amuleGUI. The context menu keeps only Clear (a
   one-shot action, not a setting to toggle from a hidden menu).

4. Cap raised from 20 to 30, matching eMule's CCustomAutoComplete default.

Optional, done anyway:
- wxComboBox::AutoComplete() wired up (eMule's ACO_AUTOSUGGEST
  equivalent), re-armed after every load/record/clear so it always
  reflects the current entry set.
- Explicitly framed as *query* history throughout (comments, tooltip)
  to distinguish it from the separate, not-yet-implemented result
  persistence that's the rest of #641.

The dedup/move-to-front/cap logic that used to live inline in
CSearchDlg::RecordSearchHistory is extracted into a pure function,
ApplySearchHistoryEntry() (SearchHistory.h/.cpp), decoupled from
wxComboBox/wxConfig specifically so it's unit-testable without a wx
event loop -- new SearchHistoryTest covers empty-list insert, reorder
of an existing term, case-insensitive dedup, empty-term no-op, and
capping (including locking in the actual 30-entry constant).

Rebased onto current master; po/ catalogs regenerated as the final
step so the diff is just the new/changed strings in sync with the tree.

Verified for real: built and ran both amule and amulegui, plus the
full unit test suite (27/27 passing, including the new
SearchHistoryTest's 7 cases).

* fix(tests): link Format.cpp + strerror_r.c into SearchHistoryTest

Same Linux/mingw-only link failure as CMuleCollectionTest/MagnetURITest
already work around: muleunit's MuleDebug.cpp needs CFormat for its
glibc backtrace path, which isn't compiled in on macOS -- so the
target linked fine locally but failed on the Ubuntu/mingw CI builds
(and the clang-tidy jobs, which build the tree first).

* i18n: regenerate po/ catalogs after rebasing onto current master

Mechanical rebase to resolve the po/ catalog conflicts against master's
#648/#650/#651 (per got3nks's note on #643) -- no source changes here,
scripts/update-po.sh output only, so the diff is just the tree back in
sync with the current strings.

* fix(search): avoid an unnamed EReadTextFile bitmask cast in history load

txtIgnoreEmptyLines|txtStripWhitespace has no single named enumerator
to cast to -- clang-tidy Tier-1 flagged it (clang-analyzer-optin.core.
EnumCastOutOfRange), correctly: EReadTextFile isn't a flag enum, so a
synthesized OR'd value is genuinely out of its declared range even
though CTextFile::ReadLines treats it as bitflags at runtime.

txtReadDefault would dodge the cast but also drops '#'-led lines,
silently eating a legitimate search term that happens to start with
one. Read unfiltered (txtReadAll, a real enumerator) and do the
trim/empty-line-drop by hand instead -- same behavior, no cast, no
lost terms.

* i18n: regenerate po/ catalogs after rebasing onto current master

Mechanical rebase to resolve the po/ conflicts against master's #642
(just merged) -- no source changes here.
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.

Feature Request: Add aich to generated magnet links and support for handling magnet links with aich hashes

2 participants