Skip to content

fix/mirror-gerrit: encode clone URL credentials and validate the creds file - #1145

Merged
keegancsmith merged 2 commits into
sourcegraph:mainfrom
GyeongHoKim:fix/#1144
Aug 25, 2026
Merged

fix/mirror-gerrit: encode clone URL credentials and validate the creds file#1145
keegancsmith merged 2 commits into
sourcegraph:mainfrom
GyeongHoKim:fix/#1144

Conversation

@GyeongHoKim

Copy link
Copy Markdown
Contributor

addPassword splices the HTTP password into the clone URL as a raw string, so a password containing a character that URL syntax reserves corrupts the result. Roughly half of Gerrit-generated passwords contain a /, which fails url.Parse in a log.Fatalf and aborts the mirror before anything is cloned, while # or ? parse cleanly into a different host. Because the splice matches on a literal ://<username>@, it also does nothing at all when the credentials file's username differs from the download scheme's, silently sending no credentials. The credentials file is separately split on every :, truncating passwords that contain a colon and panicking on a file with none.

This sets User on the already-parsed per-project URL so net/url percent-encodes it, and parses the credentials file with SplitN, reporting a malformed one instead of truncating or panicking. Scheme selection and URL assembly move into selectProjectURL and buildCloneURL so they can be tested. The credentials file now supplies the whole userinfo, so it wins when the two usernames disagree.

One caveat for verifying this. If the Gerrit URL carries an explicit port, REST authentication still fails before any of the above runs: go-gerrit's NewClient extracts credentials with a regex that only matches when a port is present and takes them as raw substrings, so it sends the percent-encoded password (andygrunwald/go-gerrit#224, fix open in #225). That path is untouched here — rootURL reaches NewClient exactly as before — but a ported instance needs the upstream fix as well.

Closes #1144.

@keegancsmith keegancsmith left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM

@keegancsmith
keegancsmith merged commit a874caf into sourcegraph:main Aug 25, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zoekt-mirror-gerrit cannot use a Gerrit HTTP password that contains /

2 participants