andygrunwald / andygrunwald/go-gerrit

Credentials in the URL are percent-decoded inconsistently depending on whether the URL has a port

Đang mở Phù hợp với người mới
#224 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Go
Star
106
Fork
54
Merge trung bình
7 giờ 21 phút
Pull request đã merge (30 ngày)
3

Mô tả

## Summary

`NewClient` extracts credentials from the URL through two different code paths, and only one of them percent-decodes them. Which path is taken depends on whether the URL contains an explicit port, so the same credentials are interpreted differently for `http://user:pass@host:8080/` and `http://user:pass@host/`.

## Details

`NewClient` first tries the `ReParseURL` regular expression (gerrit.go), and falls back to `url.Parse` if it does not match:

* Regex path (gerrit.go:110-118): `username` and `password` are taken as raw substrings of the URL, so percent-escapes are left untouched.
* `url.Parse` path (gerrit.go:128-139): `url.Userinfo.Username()` and `url.Userinfo.Password()` return percent-decoded values.

Because `ReParseURL` requires an explicit port (`:(\d+)`), a URL without a port never reaches the regex path. So the two paths disagree for any credential that contains a percent-escape.

## Reproduction

```go
func TestCredentialDecoding(t *testing.T) {
for _, rawurl := range []string{
"http://admin:se%2Fcret@localhost:8080/", // has a port -> regex path
"http://admin:se%2Fcret@gerrit.example.com/", // no port -> url.Parse path
} {
if m := gerrit.ReParseURL.FindAllStringSubmatch(rawurl, -1); len(m) > 0 && len(m[0]) > 5 {
t.Logf("%-45s regex user=%q pass=%q", rawurl, m[0][2], m[0][3])
continue
}
u, err := url.Parse(rawurl)
if err != nil {
t.Fatal(err)
}
pass, _ := u.User.Password()
t.Logf("%-45s url.Parse user=%q pass=%q", rawurl, u.User.Username(), pass)
}
}
```

Output:

```
http://admin:se%2Fcret@localhost:8080/ regex user="admin" pass="se%2Fcret"
http://admin:se%2Fcret@gerrit.example.com/ url.Parse user="admin" pass="se/cret"
```

## Expected behaviour

Both paths should produce the same credentials for the same URL. Since `url.Parse` decodes them, the regex path should decode them too, so that a user who correctly percent-encodes a credential gets the decoded value in both cases.

## Actual behaviour

With a port in the URL, the still-encoded string is stored as the secret and sent to the server verbatim (`se%2Fcret` instead of `se/cret`), so authentication fails.

## Suggested fix

Decode the two regex submatches before using them:

```go
username = submatch[2]
password = submatch[3]
if decoded, err := url.PathUnescape(username); err == nil {
username = decoded
}
if decoded, err := url.PathUnescape(password); err == nil {
password = decoded
}
```

Two notes on this:

* `url.PathUnescape` rather than `url.QueryUnescape`: `QueryUnescape` also turns `+` into a space, which would corrupt the very example password quoted in the comment above `ReParseURL` (`ZOSOKjgV/kgEkN0bzPJp+oGeJLqpXykqWFJpon/Ckg`). `PathUnescape` leaves `+` alone.
* Falling back to the raw value on error matters: `PathUnescape` fails on an unencoded `%` (for example `p%ss`), and the regex path exists precisely to accept raw credentials that `url.Parse` rejects. Ignoring the error keeps those working.

I am happy to send a PR with this change plus a table test covering both paths, if this looks like the right direction.

## Environment

* go-gerrit: master (42c7af0)
* Go: 1.16 (go.mod)

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Hướng nghiên cứu

Start in gerrit.go around ReParseURL and the NewClient URL-parsing paths. Add a table test covering percent-encoded credentials with and without an explicit port, including plus signs and malformed percent escapes, then run the relevant Go tests to verify both paths produce consistent credentials without breaking raw values.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
go
Lĩnh vực
api
Loại issue
Lỗi
Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
90/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.