The .git substring that broke but pr new for GitHub Pages repos

rust dev.to

Summer Bug Smash: Clear the Lineup 🐛🛹

This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.

The bug that only bites .github.io repos

Someone opened issue #15302 on GitButler with a clean report: running but pr new in their repo failed with HTTP 404 and the message "Failed to list open pull requests". Their remote was a GitHub Pages repo:

git@github.com:bburns-ds/bburns-test.github.io.git
Enter fullscreen mode Exit fullscreen mode

That is a completely ordinary clone URL. Nothing exotic. So why would GitButler be unable to list pull requests for a real repository the user clones every day?

Following the 404

but pr new has to turn a remote URL into an (owner, repo) pair before it can call the GitHub API. I traced the path through the crates:

  1. but pr new calls but_forge::derive_forge_repo_info(&remote_url).
  2. list_forge_reviews pulls owner and repo out of that and calls but_github's pull request list.
  3. That ends at list_open_pulls, which issues GET /repos/{owner}/{repo}/pulls.

So the 404 is GitHub telling us the request went to a repository that does not exist. The interesting question is what derive_forge_repo_info produced for a .github.io URL. I parsed the reporter's URL and got back repo = "org". Not org.github.io. Just org. The API call had become GET /repos/org/org/pulls, which of course 404s.

The .git substring trap

derive_forge_repo_info parses the URL with the git-url-parse crate. Here is the exact code that splits the path, from git-url-parse 0.6.0 (src/types/provider/generic.rs):

let (input, (user, repo)) = if input.ends_with(".git") {
    separated_pair(is_not("/"), tag("/"), take_until(".git")).parse(input)?
} else {
    separated_pair(is_not("/"), tag("/"), is_not("/")).parse(input)?
};
Enter fullscreen mode Exit fullscreen mode

Read the .ends_with(".git") branch closely. It reads the repo name with take_until(".git"). take_until stops at the first .git substring, not the terminal suffix. Most repo names never contain .git, so this is invisible almost all the time. But a GitHub Pages repo is named something.github.io, where the .github piece contains .git. So the path org/org.github.io.git gets cut at the .git inside .github and the repo name collapses to org.

The trap only springs when two things line up: the URL ends in the .git clone suffix AND the repo name contains .git earlier. Canonical GitHub clone URLs always append .git, so every Pages repo hits it. I confirmed the full parse matrix in a scratch crate against git-url-parse 0.6.0:

remote URL parsed repo should be
git@github.com:org/repo.git repo repo
git@github.com:org/org.github.io org.github.io org.github.io
git@github.com:org/org.github.io.git org org.github.io
https://github.com/org/org.github.io.git org org.github.io

Only the rows that carry both the suffix and the substring are wrong.

The fix

The buggy line lives inside a published dependency, so I cannot edit it. But I can stop feeding it the input that trips it. Look at the else branch above: for a URL that does NOT end in .git, the parser uses is_not("/"), which reads the whole repo name up to the next slash and keeps org.github.io intact.

So the fix is to strip a single trailing .git at our boundary, before parsing, in crates/but-forge/src/lib.rs:

Before:

pub fn derive_forge_repo_info(url: &str) -> Option<ForgeRepoInfo> {
    let git_url = GitUrl::parse(url).ok()?;
Enter fullscreen mode Exit fullscreen mode

After:

pub fn derive_forge_repo_info(url: &str) -> Option<ForgeRepoInfo> {
    // git-url-parse 0.6.0's GenericProvider strips the git-suffix with
    // `take_until(".git")`, which stops at the FIRST ".git" substring rather
    // than the terminal suffix. So `org/org.github.io.git` yields repo="org"
    // and every URL built from it points at the wrong repository and 404s.
    // Strip a single trailing ".git" ourselves so the parser takes its correct
    // `is_not("/")` branch. Safe because a real repo name can't end in ".git".
    let url = url.strip_suffix(".git").unwrap_or(url);
    let git_url = GitUrl::parse(url).ok()?;
Enter fullscreen mode Exit fullscreen mode

strip_suffix matters here. It removes exactly one trailing occurrence, which is the single .git clone suffix GitHub appends. Using replace(".git", "") would delete the .git inside .github and recreate the bug. It is safe to strip because GitHub does not allow a repo name to end in .git, so a trailing .git is always the clone suffix. SSH and HTTPS URLs are handled the same way because the strip runs on the raw string. The same (owner, repo) also feeds the web base URL and the commit, PR and compare links, so those are fixed too.

The test, failing then passing

I added a table-driven regression test, repo_name_containing_dotgit_is_parsed_correctly, that asserts forge, owner and repo for nine URL cases: SSH and HTTPS, each plain and with .git, plus the *.github.io family with and without the suffix.

With the fix reverted and the test kept, the suite fails on exactly the reported case:

---- tests::repo_name_containing_dotgit_is_parsed_correctly stdout ----
assertion `left == right` failed: repo for git@github.com:org/org.github.io.git
  left: "org"
 right: "org.github.io"

test result: FAILED. 103 passed;1 failed
Enter fullscreen mode Exit fullscreen mode

With the one-line fix in place, everything is green:

test tests::repo_name_containing_dotgit_is_parsed_correctly ... ok
test result: ok. 104 passed;0 failed
Enter fullscreen mode Exit fullscreen mode

That before/after pair is the proof: the test catches the real defect and the single line is what resolves it.

Verification

  • cargo test -p but-forge with the fix: 104 passed, 0 failed.
  • Same run with the fix removed: 103 passed, 1 failed on the target case.
  • cargo clippy -p but-forge --all-targets: 0 warnings.
  • cargo fmt -- --check: clean.

Links

A one-line fix, but only after the parser's first .git cut gave up its secret. The best bugs hide inside a string almost nobody thinks to name with a dot-git in the middle.

AI disclosure

AI assistance (Claude, Anthropic) was used while working on this fix. I did the diagnosis, the design, the review and the verification. I own the change. What was verified locally before submitting: cargo test -p but-forge (104 passed, plus the new test fails 1 of 104 with the fix reverted), cargo clippy -p but-forge --all-targets (0 warnings) and cargo fmt -- --check (clean).

Source: dev.to

arrow_back Back to Tutorials