Skip to content

fix(hf-artifact): parse hostname with optional port in resolve URLs - #5870

Closed
vaibhavsrv wants to merge 1 commit into
Osmantic:public-betafrom
vaibhavsrv:fix/hf-artifact-hostname-port
Closed

vaibhavsrv wants to merge 1 commit into
Osmantic:public-betafrom
vaibhavsrv:fix/hf-artifact-hostname-port

Conversation

@vaibhavsrv

Copy link
Copy Markdown
Contributor

Why this matters

In ods/scripts/download-hf-artifact.py, parse_huggingface_resolve_url() validated the target endpoint by inspecting parsed.netloc.lower(). When a resolve URL contained an explicit standard port (e.g. https://huggingface.co:443/...) or credentials, parsed.netloc included the port suffix (e.g. huggingface.co:443), failing the hardcoded set check against {"huggingface.co", "www.huggingface.co", "hf.co"} and raising a false-positive ValueError: URL is not a Hugging Face URL.

This patch normalizes host extraction using (parsed.hostname or "").lower() and restricts permitted ports to standard HTTP/HTTPS ports (None, 80, 443), rejecting non-standard ports while permitting explicit port specifications.

Validation

  • Repro baseline: Passed https://huggingface.co:443/unsloth/Llama-4-Scout-GGUF/resolve/main/model.gguf to parse_huggingface_resolve_url(); confirmed baseline raised ValueError: URL is not a Hugging Face URL.
  • Post-fix: Verified test_hf_download_helper.py passes all 4 tests with exit code 0; valid URLs with :443 parse correctly, and non-standard ports (:8443) are rejected.
  • Helper suites: 4 passed. New-test Ruff and diff checks pass; new regressions wired into Linux CI.

Overlap check

Risk / AI disclosure

AI-assisted investigation, implementation and CLI regressions. This strengthens URL parsing and host/port validation in the artifact fallback downloader, not runtime network proxies. Independent human review and platform/runtime qualification remain gates. No running configuration, deployment or upstream merge changed.

Follow-up integration evidence

Composed with #5863 at 1de196a without conflicts. Production and test diffs passed together; artifact download and cache revision contracts remain intact.
Backlog composition was local-only (production/test diffs, excluding workflow/Makefile wiring); it is not an upstream merge or independent human approval. Declared live-review gates remain open.

@Lightheartdevs

Copy link
Copy Markdown
Collaborator

Thanks for this contribution. public-beta was promoted into main on 2026-09-24 and no longer receives changes, so we're closing pull requests that target it. This isn't a judgment on the change itself. If it's still needed, please rebase onto main and open a focused PR. See #7253 for details and the contribution policy.

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.

2 participants