Follow-up from PR #789 review panel -- "Follow-up A: dedup key case sensitivity" per the merged panel verdict. An inline NOTE already marks the site:
src/apm_cli/deps/github_downloader.py:771-775
# NOTE: dedup key is case-sensitive. GitHub/Bitbucket hostnames
# are case-insensitive per RFC, so "Example.com" and
# "example.com" dedup as distinct identities -- worst case is a
# duplicate warning. Follow-up issue tracks normalization.
warn_key = (dep_host, repo_url_base, dep_port)
Describe the bug
The PR #789 cross-protocol-fallback warning deduplicates via self._fallback_port_warned: set keyed by (dep_host, repo_url_base, dep_port). DNS hostnames are case-insensitive per RFC 4343, so a manifest that references the same host under different casing (or with different hosts normalised by upstream rewriters -- git config url.<base>.insteadOf, registry-proxy rewrites, etc.) will warn more than once about the same logical dependency.
Worst case is cosmetic (a duplicate [!] line); there is no correctness or security fallout. But the whole point of the dedup set is "exactly once per dep identity," and host casing is the only axis where identity currently leaks.
To Reproduce
- Construct an
apm.yml that references the same host via two casings, e.g.:
dependencies:
apm:
- git: ssh://git@bitbucket.example.com:7999/team/repo.git
- git: ssh://git@Bitbucket.Example.com:7999/team/other-repo.git
apm install --allow-protocol-fallback
- Observe two distinct
[!] warnings for what users think of as the same host on the same port.
(An easier repro is to install the same dep twice programmatically via two resolution passes with different casings, but the manifest form above is the one a user could hit organically.)
Expected behavior
Dedup key should normalise the host to a canonical casing before insertion, matching the (host.lower() if host else host, ...) convention used by AuthResolver._cache at core/auth.py:220 (established in #788). One-line fix:
warn_key = (dep_host.lower() if dep_host else dep_host, repo_url_base, dep_port)
Plus a regression test covering the two-casings-one-warning invariant.
Environment
- Not environment-dependent.
- Trigger surface: only when
--allow-protocol-fallback (or APM_ALLOW_PROTOCOL_FALLBACK=1) is active and dep_ref.port is not None and at least one dep shows up under two casings in the same install run.
Additional context
- The inline NOTE at the construction site (lines 771-774) was added exactly as a placeholder for this issue; resolving the issue should also remove the NOTE.
- Related:
AuthResolver._cache already lowercases its host axis; aligning the dedup key with that precedent keeps cache-identity semantics uniform across the module.
- Scope-limited: one-line fix + one test. No schema or UX impact.
Refs
Follow-up from PR #789 review panel -- "Follow-up A: dedup key case sensitivity" per the merged panel verdict. An inline
NOTEalready marks the site:Describe the bug
The PR #789 cross-protocol-fallback warning deduplicates via
self._fallback_port_warned: setkeyed by(dep_host, repo_url_base, dep_port). DNS hostnames are case-insensitive per RFC 4343, so a manifest that references the same host under different casing (or with different hosts normalised by upstream rewriters --git config url.<base>.insteadOf, registry-proxy rewrites, etc.) will warn more than once about the same logical dependency.Worst case is cosmetic (a duplicate
[!]line); there is no correctness or security fallout. But the whole point of the dedup set is "exactly once per dep identity," and host casing is the only axis where identity currently leaks.To Reproduce
apm.ymlthat references the same host via two casings, e.g.:apm install --allow-protocol-fallback[!]warnings for what users think of as the same host on the same port.(An easier repro is to install the same dep twice programmatically via two resolution passes with different casings, but the manifest form above is the one a user could hit organically.)
Expected behavior
Dedup key should normalise the host to a canonical casing before insertion, matching the
(host.lower() if host else host, ...)convention used byAuthResolver._cacheatcore/auth.py:220(established in #788). One-line fix:Plus a regression test covering the two-casings-one-warning invariant.
Environment
--allow-protocol-fallback(orAPM_ALLOW_PROTOCOL_FALLBACK=1) is active anddep_ref.port is not Noneand at least one dep shows up under two casings in the same install run.Additional context
AuthResolver._cachealready lowercases its host axis; aligning the dedup key with that precedent keeps cache-identity semantics uniform across the module.Refs