SKETCH: confirm credential with a known-good probe before rejecting on HTTP 400 - #2089
Draft
tyrielv wants to merge 1 commit into
Draft
SKETCH: confirm credential with a known-good probe before rejecting on HTTP 400#2089tyrielv wants to merge 1 commit into
tyrielv wants to merge 1 commit into
Conversation
…n 400 Design proposal / prototype - alternative to unconditionally keeping the credential on HTTP 400. Posting for discussion, not for merge as-is. Idea: a 400 is normally a request or formatting problem, not an expired credential (an expired or invalid credential returns 401 or 302). The one 400 that can indicate a credential problem is a completely missing Basic auth header. Rather than guess, when a 400 arrives we re-send the SAME credential to a known-good, auth-enforced endpoint and only reject the credential if that probe ALSO fails authentication. Decisive signal: the probe rejects the credential ONLY on 401 or 302. Any other probe status - including 200 and 404 - proves the credential got past auth, so we keep it. A 404 counts as success: we reached "object not found" past the auth gate. Sketch details: - HttpRequestor.SendRequest: 400 no longer shares the reject branch with 401/302. It rejects only when CredentialProbeConfirmsAuthFailure returns true. - HttpRequestor.GetCredentialProbeUri (virtual): returns a known-good probe URI built from a constant we control, never from the request input that caused the 400. Default null means "cannot probe" - the caller then keeps the credential. - GitObjectsHttpRequestor overrides it to GET the git empty-tree object from the cache server's own objects endpoint, so the probe exercises the same host and auth path that returned the 400. - TryProbeCredential: one-shot, no-retry GET carrying the same Basic auth header, separate from SendRequest so it never re-enters the 400/401 handling. - GVFSConstants.WellKnownObjects.EmptyTreeSha: the fixed empty-tree SHA. Open questions for review: probe host choice (cache objects vs origin /info/refs), memoizing the probe result per credential to avoid one probe per 400, and whether the simpler "never reject on 400" change is sufficient on its own. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
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.
Idea
A 400 is normally a request or formatting problem, not an expired credential — an expired or invalid credential always returns 401 or 302. The one 400 that can indicate a credential problem is a completely missing Basic auth header ("A valid Basic Authorization header is required.").
Rather than guess, when a 400 arrives we re-send the same credential to a known-good, auth-enforced endpoint and reject the credential only if that probe also fails authentication.
Decisive signal
The probe rejects the credential only on 401 or 302. Any other probe status — including 200 and 404 — proves the credential got past auth, so we keep it. A 404 counts as success: we reached "object not found" past the auth gate.
Sketch details
HttpRequestor.SendRequest— 400 no longer shares the reject branch with 401/302. It rejects only whenCredentialProbeConfirmsAuthFailurereturns true.HttpRequestor.GetCredentialProbeUri(virtual) — returns a probe URI built from a constant we control, never from the request input that caused the 400. Defaultnullmeans "cannot probe" → the caller keeps the credential.GitObjectsHttpRequestoroverrides it to GET the git empty-tree object from the cache server's own objects endpoint, so the probe exercises the same host and auth path that returned the 400.TryProbeCredential— one-shot, no-retry GET carrying the same Basic auth header, deliberately separate fromSendRequestso it never re-enters the 400/401 handling (no recursion, no retry, no circuit-breaker interaction).GVFSConstants.WellKnownObjects.EmptyTreeSha— the fixed empty-tree SHA4b825dc642cb6eb9a060e54bf8d69288fbee4904.Why
/gvfs/configis not the probe/gvfs/configis already used as the anonymous probe during auth init — on an anonymous-capable repo it returns 200 without any credential, so a 200 there would not prove the token is valid. The probe must hit an endpoint where auth is actually enforced.Tests
GVFS.UnitTests/Http/CredentialProbeDecisionTests.cscovers the decisive-signal rule: probe 401/302 → reject; probe 200/404/400 → keep. (The network probe itself is not unit-tested — it needs a live endpoint.)Open questions for review
/info/refs.Relationship to #2088
#2088 is the minimal fix (drop 400 from the reject path). This is the alternative that keeps a confirmed auth-failure path for 400 via the probe. They are mutually exclusive designs — pick one.