Conversation
metif12
force-pushed
the
fix/path-to-uri-unc-authority
branch
from
October 3, 2026 11:32
f77bed1 to
8518034
Compare
`uri_to_path` already resolves a `file://host/...` URI to a `//host/share/...`
UNC path, but `path_to_uri` then treated that as an ordinary absolute path and
emitted four slashes:
client sends file://server/share/proj/main.v
uri_to_path //server/share/proj/main.v
path_to_uri file:////server/share/proj/main.v
The share ends up in the path instead of the authority. That is not a valid
file URI (RFC 8089 puts the host in the authority), and it breaks the round
trip in a way that matters: VLS keys open buffers, the index, and every
published diagnostic and code lens by URI, so the URI it derives for a file on
a network share never matches the one the client sent for the same file.
Encode the host as the authority instead:
path_to_uri('//server/share/proj/main.v') == 'file://server/share/proj/main.v'
Percent-encoding of the path component is unchanged, so a share with a space
still encodes correctly and still round-trips. Single-slash absolute paths and
Windows drive paths are untouched, so POSIX and local Windows behaviour is
identical.
metif12
force-pushed
the
fix/path-to-uri-unc-authority
branch
from
October 3, 2026 11:46
8518034 to
8c68ac9
Compare
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
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.
Problem
uri_to_pathresolves afile://host/...URI to a//host/share/...UNC pathcorrectly, and there is already a test for it:
path_to_uridid not invert that. It treated the resulting UNC path as anordinary absolute path and emitted four slashes:
The host ends up in the path component instead of the authority. Per RFC 8089 a
UNC path's host belongs in the authority —
file://server/share/proj/main.v—and
file:////server/share/...is not a valid file URI for it.The round trip breaking is the part that actually bites. VLS keys open buffers,
the index, and every published diagnostic and code lens by URI, so the URI it
derives for a file on a network share never matches the URI the editor sent for
that same file. Reproduced directly:
This is a Windows-only path:
\\server\share\...has no POSIX equivalent that VLSmeets in practice.
Fix
Encode the host as the authority:
placed after
os.to_slashand before the drive-letter branch, so a single-slashabsolute path and a Windows drive path still take exactly the path they did
before. This is symmetric with what
uri_to_pathalready does with a non-localauthority, so the pair now round-trips.
Test
test_path_to_uri_keeps_the_authority_of_a_unc_pathcovers the round trip fromthe client's URI, asserts the result is not
file:////, checks that a sharecontaining a space is still percent-encoded and still round-trips, and pins
POSIX and drive-letter paths so the change cannot silently widen. There was no
path_to_uricoverage for a UNC path before.Validation
V
0137eb5(thevlang/vrevision CI builds from), Windows.interop_test.von this branch:OK.For the regression check I applied the same two-file change on top of
#526 +
#527 +
#528 +
#529, which are green together on
master's current state, and ran the whole suite:v fmt -verify .also exits 0. URI keys are used pervasively —app.open_files,the index, diagnostics, code lens, workspace edits — so this was worth checking
against the full suite rather than only the new test.
Note on the diff
As with #529, this branch runs
v fmtover the two files it touches, sincemasteris notv fmt-clean (#528).