Release M279.3 - #2092
Open
tyrielv wants to merge 12 commits into
Open
Conversation
When a user process reads a virtualized placeholder whose stored
content-id is corrupt - specifically 40 NUL bytes instead of a hex SHA -
GVFS builds a loose-object path from it and Path.Combine throws
System.ArgumentException ("Illegal characters in path").
ArgumentException is not in RetryWrapper.IsHandlableException, so it
bypasses both the retry logic and the download fallback in
GVFSGitObjects.TryCopyBlobContentStream and propagates to the
virtualizer's outer catch, which returns FileNotAvailable to ProjFS. The
placeholder can never hydrate, so the failing read repeats forever - a
retry storm. This is the #1 blob-hydration failure cause on the LKG field
build 1.0.26014.1 (38 machines; ~61 machines / ~6.2K events across 30d;
one machine emitted ~2.49M error events).
This is a corrupt content-id, NOT GVFSConstants.AllZeroSha: AllZeroSha is
40 ASCII '0' characters, which yields directory "00" and does not throw.
Reject a malformed SHA before it is turned into a path:
- GitRepo.GetLooseBlobState returns LooseBlobState.Invalid (a clean,
non-retryable miss) for a SHA that is not 40 hex characters, so
Path.Combine can never throw here again.
- GitRepo.LooseObjectExists guards the same Path.Combine.
- GVFSGitObjects.TryCopyBlobContentStream short-circuits a malformed SHA
before the retry loop, so a bogus SHA never triggers a doomed 404
download or a retry storm.
- SHA1Util.IsValidShaFormat is now null-safe; SHA1Util.ToLoggableShaString
renders the bad value with non-hex characters escaped so telemetry stays
greppable and free of control characters.
- WindowsFileSystemVirtualizer routes the request's logged sha through
ToLoggableShaString, so a malformed content-id can no longer enter
telemetry with raw NUL/control bytes at the terminal hydration-failure
error either (a no-op for a valid hex SHA).
All three guard sites emit the same greppable Warning event
(*_MalformedBlobSha) at Warning level with no unhandled exception. Per an
existing decision this case stays telemetry category "Unexpected"; no new
BlobHydrationFailureCategory is added.
Stacked on #2071 (tyrielv/split-hydration-enum-telemetry): this branch is
rebased onto it, so #2071's out BlobHydrationFailureCategory parameter is
honored - the malformed-SHA short-circuit sets failureCategory =
Unexpected, so the virtualizer's terminal telemetry tags the case exactly
as before (it no longer reaches the outer catch because it no longer
throws). This PR must NOT merge before #2071; after #2071 lands, rebase
onto master.
Unit tests assert that a 40-NUL-byte SHA, a 40-char SHA with an embedded
path-illegal character, and other malformed SHAs return false from both
GitRepo.TryCopyBlobContentStream and GVFSGitObjects.TryCopyBlobContentStream
with no ArgumentException (Assert.DoesNotThrow), that no download/retry is
attempted, and that the out category is Unexpected.
Assisted-by: Claude Opus 4.8
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
….vfs.0.8 Update default Microsoft Git version to v2.55.0.vfs.0.8
When an on-demand loose-blob download fails, GVFS emits a terminal telemetry error with a BlobHydrationFailureCategory. The DownloadFailed category is the transient or unclassified bucket. It collapses genuine auth failures (401, 400, 302) and transient failures (timeout 408, 5xx, pool-exhaustion 503) into one value. The HTTP status of the failing download is known in the code, but it only reaches the on-box log, not shipped telemetry. So telemetry cannot tell a real auth failure apart from a transient one. Carry the HTTP status of the last download attempt to the terminal failure event through an internal DownloadAttemptResult type. Add HttpStatusCode and HttpStatusName to the event metadata only when the failure is attributable to the download itself (DownloadFailed or ObjectNotOnServer), so an earlier attempt's status cannot attach to a later local-IO or copy failure. The public TryDownloadAndSaveObject return type does not change. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…Repo git_config_get_string returns a borrowed pointer whose lifetime is tied to the config object, so libgit2 only permits it on a snapshot (read-only) config. LibGit2Repo.GetConfigString called it on the live config returned by git_repository_config, which fails with "get_string called on a live config object". The read then threw LibGit2Exception, and callers silently fell back to their default value instead of honoring the configured setting. Take a git_config_snapshot of the live config and read the string from the snapshot, freeing the snapshot afterward. Also stop marshalling the result as an out string. git_config_get_string returns a borrowed const char* owned by the config; the interop marshaller would free that pointer with CoTaskMemFree, a mismatched-allocator free of memory libgit2 still owns, corrupting the heap. Retrieve the value as an IntPtr and copy it with Marshal.PtrToStringUTF8, which never frees the borrowed pointer. This matches the manual marshalling already used by GitConfigEntry. Not-found still returns null so the documented default applies without a spurious error. git_config_get_bool is unaffected (it parses the value rather than returning a borrowed pointer), so GetConfigBool is left unchanged. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Add LibGit2ConfigTests, a functional test that exercises the real libgit2 (git2.dll) config-read path in LibGit2Repo against a plain on-disk git repository. It creates a temp repo, sets a string and a bool config value, then reads them back through LibGit2Repo and asserts a missing key returns null. No unit test can cover this: the unit tests mock the native layer, so the "get_string called on a live config object" failure only manifests through the real P/Invoke. This test fails before the snapshot/marshalling fix and passes after, guarding the regression. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
The malformed-SHA guard in TryCopyBlobContentStream only covers the blob hydration path. Two other callers reach the object download directly, so a corrupt (NUL-byte) placeholder SHA from them still reached the network: - the git.exe read-object hook (RequestSource.NamedPipeMessage, via InProcessMount), and - the gitattributes GVFSVerb (RequestSource.GVFSVerb). On .NET Framework the local Path.Combine threw ArgumentException on such a value, so the download was never reached. On modern .NET (which 2.0 runs) Path.Combine no longer validates path characters, so the malformed SHA silently misses the local object store and is sent to the cache server. The Application Gateway rejects the malformed URL with HTTP 400, and GVFS then treats the 400 as an auth failure and erases a valid credential, producing a credential-prompt storm (ICM 850075166). Reject a malformed object SHA at the download chokepoint (GVFSGitObjects.TryDownloadAndSaveObject, next to the existing AllZeroSha guard) for every request source, before any request is built, and emit the same greppable *_MalformedBlobSha Warning as the other guards. Also correct the GetLooseBlobState comment: the ArgumentException it described is .NET-Framework-only, so validating (not relying on the throw) is what makes the guard correct on modern .NET. Unit test asserts a malformed SHA returns Error from TryDownloadAndSaveObject across FileStreamCallback / NamedPipeMessage / GVFSVerb, never reaches the network (download call count stays 0), and is logged. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…-getstring Read git config strings via a config snapshot in LibGit2Repo
Fail blob hydration cleanly on a malformed (NUL-byte) placeholder SHA
Record HTTP status code on blob-hydration failure telemetry
The malformed-SHA guard in TryDownloadAndSaveObject returned the bare DownloadAndSaveObjectResult enum. The method now returns DownloadAttemptResult (the enum plus an HTTP status code), so the build failed with CS0029. Wrap the error in a DownloadAttemptResult with a null status code, matching the AllZeroSha guard just below it. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…esult Fix build break: return DownloadAttemptResult from the malformed-SHA guard
tyrielv
enabled auto-merge
August 14, 2026 22:41
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
2 participants
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.
Changes: