From ed9a3ca2a7289ecd8d776af4db1b207e7176d8e5 Mon Sep 17 00:00:00 2001 From: Daksha1611 Date: Fri, 11 Sep 2026 23:57:18 +0530 Subject: [PATCH] Do not mistake a URL port for a LoRA alpha suffix The optional :alpha suffix of a --source_loras entry was located with rfind(':') over the whole source string, guarded only by a check that the text after the colon does not start with "//". That guard covers the scheme colon of a URL with no port, but not the port colon itself. For an entry with a port and no explicit alpha, such as xray=https://registry.internal:8080/loras/f.safetensors rfind(':') lands on the port colon, alphaStr becomes "8080/loras/f.safetensors", ovms::stof rejects it because it requires the whole string to be consumed, and startup aborts with the misleading message "Invalid alpha value '8080/loras/f.safetensors'". Restrict the alpha colon to the final path segment. The scheme colon, the port colon and a Windows drive letter all precede the last separator, so all three are excluded structurally rather than by special-casing. The "//" check is dropped because it is now unreachable: nothing after the last separator can contain a path separator, so alphaStr can never start with one. Note this only affected entries without an explicit alpha - with ":0.45" appended, rfind() happened to land on the alpha colon and parsing worked. Tests: adds UrlLoraWithPortWithoutAlpha (the failing case) and UrlLoraWithPortAndAlpha (the case that already worked, pinned against regression). --- .../image_generation_graph_cli_parser.cpp | 23 ++++++++------ src/test/lora_graph_export_test.cpp | 31 +++++++++++++++++++ 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/graph_export/image_generation_graph_cli_parser.cpp b/src/graph_export/image_generation_graph_cli_parser.cpp index 5b5177f432..d8c464bbf1 100644 --- a/src/graph_export/image_generation_graph_cli_parser.cpp +++ b/src/graph_export/image_generation_graph_cli_parser.cpp @@ -230,19 +230,22 @@ void ImageGenerationGraphCLIParser::prepare(ServerSettingsImpl& serverSettings, LoraAdapterSettings adapter; adapter.alias = alias; - // Parse optional :alpha suffix + // Parse optional :alpha suffix. Only a colon in the final path segment can be an + // alpha delimiter, which keeps the scheme colon of "https://...", the port colon of + // "https://host:8080/file.safetensors" and a Windows drive letter ("C:\\...") from + // being misread as one. A "//" check is no longer needed: everything after the last + // separator is free of path separators, so alphaStr can never start with one. + const size_t lastSeparator = source.find_last_of("/\\"); + const size_t segmentStart = (lastSeparator == std::string::npos) ? 0 : lastSeparator + 1; auto lastColon = source.rfind(':'); - if (lastColon != std::string::npos && lastColon > 1) { + if (lastColon != std::string::npos && lastColon > 1 && lastColon >= segmentStart) { 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 + "'"); - } - adapter.alpha = alpha.value(); - source = source.substr(0, lastColon); + auto alpha = ovms::stof(alphaStr); + if (!alpha.has_value()) { + throw std::invalid_argument("Invalid alpha value '" + alphaStr + "' in --source_loras entry: '" + entry + "'"); } + adapter.alpha = alpha.value(); + source = source.substr(0, lastColon); } // Detect source type if (source.substr(0, 8) == "https://" || source.substr(0, 7) == "http://") { diff --git a/src/test/lora_graph_export_test.cpp b/src/test/lora_graph_export_test.cpp index 00d91d1d05..db6ef76ccd 100644 --- a/src/test/lora_graph_export_test.cpp +++ b/src/test/lora_graph_export_test.cpp @@ -585,6 +585,37 @@ TEST(ImageGenCLILoraParsingTest, UrlLoraWithoutAlphaPreservesDefault) { EXPECT_FALSE(graphSettings.loraAdapters[0].alpha.has_value()); } +// A URL may carry an explicit port. Without an alpha suffix the port colon is the last +// colon in the entry, so it must not be taken for an alpha delimiter. +TEST(ImageGenCLILoraParsingTest, UrlLoraWithPortWithoutAlpha) { + ovms::ServerSettingsImpl serverSettings; + serverSettings.serverMode = ovms::HF_PULL_MODE; + ovms::HFSettingsImpl hfSettings; + hfSettings.sourceLoras = "pokemon=https://registry.internal:8080/loras/weights.safetensors"; + ovms::ImageGenerationGraphCLIParser parser; + parser.prepare(serverSettings, hfSettings, "test_model"); + auto& graphSettings = std::get(hfSettings.graphSettings); + ASSERT_EQ(graphSettings.loraAdapters.size(), 1); + EXPECT_EQ(graphSettings.loraAdapters[0].sourceType, ovms::LoraSourceType::DIRECT_URL); + EXPECT_EQ(graphSettings.loraAdapters[0].sourceLora, "https://registry.internal:8080/loras/weights.safetensors"); + EXPECT_EQ(graphSettings.loraAdapters[0].safetensorsFile.value(), "weights.safetensors"); + EXPECT_FALSE(graphSettings.loraAdapters[0].alpha.has_value()); +} + +TEST(ImageGenCLILoraParsingTest, UrlLoraWithPortAndAlpha) { + ovms::ServerSettingsImpl serverSettings; + serverSettings.serverMode = ovms::HF_PULL_MODE; + ovms::HFSettingsImpl hfSettings; + hfSettings.sourceLoras = "pokemon=https://registry.internal:8080/loras/weights.safetensors:0.45"; + ovms::ImageGenerationGraphCLIParser parser; + parser.prepare(serverSettings, hfSettings, "test_model"); + auto& graphSettings = std::get(hfSettings.graphSettings); + ASSERT_EQ(graphSettings.loraAdapters.size(), 1); + EXPECT_EQ(graphSettings.loraAdapters[0].sourceLora, "https://registry.internal:8080/loras/weights.safetensors"); + ASSERT_TRUE(graphSettings.loraAdapters[0].alpha.has_value()); + EXPECT_FLOAT_EQ(graphSettings.loraAdapters[0].alpha.value(), 0.45f); +} + TEST(ImageGenCLILoraParsingTest, NPURejectsMultiLoraWithoutComposites) { ovms::ServerSettingsImpl serverSettings; serverSettings.serverMode = ovms::HF_PULL_MODE;