openvinotoolkit / openvinotoolkit/model_server

--source_loras misreads a URL port as the :alpha suffix and refuses to start

Open
#4,545 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
931
Forks
277
Avg merge
2d 13h
Merged PRs (30d)
68

Description

Describe the bug

--source_loras rejects a LoRA URL that carries an explicit port, with a misleading "Invalid alpha value" error, and the server fails to start.

The optional :alpha suffix is located with rfind(':') over the whole source string, before the source type is known — src/graph_export/image_generation_graph_cli_parser.cpp:232-245:

// Parse optional :alpha suffix
auto lastColon = source.rfind(':');
if (lastColon != std::string::npos && lastColon > 1) {
    std::string alphaStr = source.substr(lastColon + 1);
    // Skip protocol colons (https:// or http://)
    if (alphaStr.substr(0, 2) != "//") {
        auto alpha = ovms::stof(alphaStr);
        if (!alpha.has_value()) {
            throw std::invalid_argument("Invalid alpha value '" + alphaStr + "' in --source_loras entry: '" + entry + "'");
        }
        ...

The != "//" guard covers the scheme colon of a URL with no port. It does not cover the port colon. For

xray=https://registry.internal:8080/loras/f.safetensors

rfind(':') lands on the port colon, alphaStr becomes 8080/loras/f.safetensors, and ovms::stof rejects it because it requires the whole string to be consumed (src/stringutils.cpp:203-205). The entry is then reported as having an invalid alpha value.

The lastColon > 1 guard correctly protects a Windows drive letter (C:\...), but nothing protects the URL authority component.

Scope: this only affects entries without an explicit alpha suffix. With :0.45 appended, rfind() happens to land on the alpha colon and parsing works, which is presumably why it was not caught — the existing URL tests (UrlLoraWithAlpha, UrlLoraWithoutAlphaPreservesDefault in src/test/lora_graph_export_test.cpp) all use default-port https://huggingface.co/... URLs, as do the CLI cases in src/test/ovmsconfig_test.cpp (:592, :610, :628, :1873) and src/test/pull_hf_model_test.cpp:2586.

To Reproduce

ovms --pull --source_model <sdxl-model> --model_repository_path /tmp/repo \
     --task image_generation \
     --source_loras "xray=https://registry.internal:8080/loras/DD-xray-v1.safetensors"

Result:

Invalid alpha value '8080/loras/DD-xray-v1.safetensors' in --source_loras entry: 'xray=https://registry.internal:8080/loras/DD-xray-v1.safetensors'

Behaviour of the current parsing block across inputs (ovms::stof copied verbatim):

https://huggingface.co/org/repo/resolve/main/f.safetensors   -> OK, no alpha
https://registry.internal:8080/loras/f.safetensors           -> THROW: Invalid alpha value '8080/loras/f.safetensors'
http://localhost:9000/f.safetensors                          -> THROW: Invalid alpha value '9000/f.safetensors'
https://host/f.safetensors:0.8                               -> OK, alpha 0.8
https://registry.internal:8080/loras/f.safetensors:0.45      -> OK, alpha 0.45

Expected behavior

alias=https://host:port/path/file.safetensors is accepted as DIRECT_URL with the default alpha, and alias=https://host:port/path/file.safetensors:0.45 is accepted with alpha 0.45.

Logs

Startup aborts before logging begins; the std::invalid_argument message above is what reaches the console.

Configuration

  1. OVMS version: main @ fadb3314
  2. --task image_generation, --source_loras pointing at a LoRA served from a host with an explicit port (internal registry, local mirror, localhost:port during testing)
  3. CPU
  4. N/A
  5. Any .safetensors LoRA reachable over a non-default port

Additional context

Suggested fix: restrict the alpha colon to the final path segment — compute source.find_last_of("/\\") and require the colon to fall at or after it. The scheme colon, the port colon and a Windows drive letter all precede the last separator, so all three are excluded structurally instead of by special-casing. The "//" check then becomes unreachable, since nothing after the last separator can contain a path separator.

I have a patch for this and will open a PR shortly.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/graph_export/image_generation_graph_cli_parser.cpp:232-245 and review the existing URL cases in src/test/lora_graph_export_test.cpp and src/test/ovmsconfig_test.cpp. Add regression coverage for URLs with explicit ports, both without an alpha and with an alpha suffix. Done means both forms are accepted with the expected default or explicit alpha.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
cli, testing
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.