Do not mistake a URL port for a LoRA alpha suffix - #4546
Open
Daksha1611 wants to merge 1 commit into
Open
Conversation
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).
Daksha1611
marked this pull request as ready for review
September 12, 2026 07:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠 Summary
Fixes #4545.
The optional
:alphasuffix of a--source_lorasentry was found withrfind(':')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, soaborts startup with
Invalid alpha value '8080/loras/f.safetensors'.Restricts the alpha colon to the final path segment, via
source.find_last_of("/\\"). The scheme colon, the port colon and a Windows drive letter all sit before the last separator, so all three are excluded structurally instead of by special-casing. The//check is dropped because it is now unreachable — nothing after the last separator can contain a path separator.lastColon > 1is kept so a source with no separator behaves as before.This only affected entries without an explicit alpha; with
:0.45appended,rfind()happened to land on the alpha colon and parsing worked. AddsUrlLoraWithPortWithoutAlphafor the broken case andUrlLoraWithPortAndAlphato pin the one that already worked.I have no OVMS build container available, so this is not compiled against the full tree and CI will need to confirm the build. I did check the rewritten block in isolation against the HF repo, URL, posix path and Windows drive-letter forms, with and without an alpha suffix; only the two port cases change behaviour. Draft for that reason.
🧪 Checklist