diff --git a/csharp/src/HgResume.Api/HgRunner.cs b/csharp/src/HgResume.Api/HgRunner.cs index d256e02..c3c8f44 100644 --- a/csharp/src/HgResume.Api/HgRunner.cs +++ b/csharp/src/HgResume.Api/HgRunner.cs @@ -182,7 +182,14 @@ private List GetRevisionsInternal(int offset, int quantity, string? bran var (tip, _) = ProcessRunner.RunSync(RepoPath, "hg", "tip", "--template", "{rev}:{branches}\n"); if (tip.Count == 1 && tip[0].StartsWith("-1")) { - // e.g. '-1:default' -> '0:default' to signal the empty repo + // Empty repo (hg init, zero changesets). At offset 0 we emit '0:' (from + // '-1:') as the sentinel callers expect; past offset 0 there is nothing more, + // so return empty. Returning the sentinel for every offset would make paginating + // callers (e.g. IsValidBase) loop forever, since they never see an empty page. + if (offset > 0) + { + return new List(); + } tip[0] = Regex.Replace(tip[0], "^-1", "0"); return tip; } @@ -205,7 +212,7 @@ public bool IsValidBase(IReadOnlyList hashes) var revisions = GetRevisions(i, q); if (revisions.Count == 0) { - return false; + return false; // paged past the last revision without matching every hash } foreach (var hashAndBranch in revisions) { @@ -217,8 +224,16 @@ public bool IsValidBase(IReadOnlyList hashes) if (foundHash >= hashes.Count) break; } } + // A page shorter than the requested quantity means hg returned everything it had, so this + // was the last page. Stop rather than advancing the offset again: this guarantees the loop + // terminates even if GetRevisions ever returns a fixed non-empty page regardless of offset + // (the empty-repo '0:' sentinel bug, or any similar future quirk). + if (revisions.Count < q) + { + break; + } i += q; } - return true; + return foundHash >= hashes.Count; } } diff --git a/csharp/test/HgResume.HttpTests/PullFacts.cs b/csharp/test/HgResume.HttpTests/PullFacts.cs index 8e1f353..9846bd6 100644 --- a/csharp/test/HgResume.HttpTests/PullFacts.cs +++ b/csharp/test/HgResume.HttpTests/PullFacts.cs @@ -159,6 +159,54 @@ public void PullBundleChunk_EmptyRepositoryReturnsNoChanges() Assert.Equal("NOCHANGE", r.Status); } + [Fact] + public async Task PullBundleChunk_EmptyRepoWithNonZeroBaseHash_FailsWithoutHanging() + { + // Regression: IsValidBase looped forever on an empty (hg init, zero-changeset) repo whenever the + // requested baseHash was anything other than "0". GetRevisions returns ["0:"] for the empty repo + // regardless of offset, so the hash is never found and IsValidBase keeps advancing the offset and + // re-querying forever, hanging the request. Contrast with PullBundleChunk_EmptyRepositoryReturnsNoChanges, + // which passes baseHash "0" and short-circuits before the loop. + _fx.SeedRepo("emptyHgRepo.zip"); + string tx = nameof(PullBundleChunk_EmptyRepoWithNonZeroBaseHash_FailsWithoutHanging); + Api.FinishPullBundle(tx); + + // Run on a background task with a timeout so the bug surfaces as a fast, clear failure rather than + // hanging until the HTTP client's 120s timeout (or forever, once the fix removes that safety net). + var call = Task.Run(() => Api.PullBundleChunk("emptyHgRepo", new[] { "fakehash" }, 0, 50, tx)); + var finished = await Task.WhenAny(call, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.True(finished == call, + "PullBundleChunk against an empty repo with a non-zero baseHash did not return within 30s — " + + "IsValidBase is looping forever."); + + // An unknown baseHash is invalid, so the API should reject it the same way it does on a non-empty repo. + var r = await call; + Assert.Equal("FAIL", r.Status); + } + + [Fact] + public async Task PullBundleChunk_NonEmptyRepoMissingHashAcrossPages_FailsWithoutHanging() + { + // manyRevsHgRepo has 205 revisions, more than IsValidBase's page size (q = 200). A baseHash that + // does not exist forces the pagination loop past the first full page (offset 0 -> 200) and onto a + // short final page, exercising the offset-advancement + short-page-break branch that the + // single-page PullBundleChunk_InvalidHash_FailCode test never reaches. It must terminate with FAIL + // rather than paging forever. + _fx.SeedRepo("manyRevsHgRepo.zip"); + string tx = nameof(PullBundleChunk_NonEmptyRepoMissingHashAcrossPages_FailsWithoutHanging); + Api.FinishPullBundle(tx); + + // Guard with a timeout so a non-terminating loop surfaces as a fast, clear failure. + var call = Task.Run(() => Api.PullBundleChunk("manyRevsHgRepo", new[] { "ffffffffffff" }, 0, 50, tx)); + var finished = await Task.WhenAny(call, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.True(finished == call, + "PullBundleChunk against a >200-revision repo with a missing baseHash did not return within 30s — " + + "IsValidBase pagination is not terminating."); + + var r = await call; + Assert.Equal("FAIL", r.Status); + } + [Fact] public void PullBundleChunk_LongMakeBundle_InProgressCode() { diff --git a/csharp/test/HgResume.HttpTests/data/manyRevsHgRepo.zip b/csharp/test/HgResume.HttpTests/data/manyRevsHgRepo.zip new file mode 100644 index 0000000..db1400e Binary files /dev/null and b/csharp/test/HgResume.HttpTests/data/manyRevsHgRepo.zip differ