cloudwego / cloudwego/hertz

URI.Update misparses relative redirect Location whose query embeds an absolute URL (e.g. /login?redirect=https://host/), breaking client DoRedirects

Open Beginner friendly
#1,539 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
7.4k
Forks
643
Avg merge
14h 5m
Merged PRs (30d)
2

Description

### Describe the bug

`protocol.URI.Update` / `UpdateBytes` decides "absolute uri" by searching for `//` **anywhere** in the new URI string:

```go
// pkg/protocol/uri.go (v0.10.6)
func (u *URI) updateBytes(newURI, buf []byte) []byte {
...
n := bytes.Index(newURI, bytestr.StrSlashSlash)
if n >= 0 {
// absolute uri
```

A *relative* redirect `Location` whose **query string embeds an absolute URL** — the classic SSO login pattern `Location: /login/redirect_to_sso?redirect=https://example.com/` — contains `//` inside the query, so it is misclassified as an absolute URI. The whole string is then re-parsed from scratch, producing an empty host (and the scheme downgrades from `https` to `http`).

Since `client.DoRequestFollowRedirects` resolves every hop through `getRedirectURL` → `URI.UpdateBytes`, `Client.DoRedirects` fails on the second hop for any server that answers with this very common redirect shape. Depending on the dialer, the failure surfaces as `dial tcp :80: connection refused` or `missing required Host header in request`.

### Minimal reproduction (no network needed)

```go
u := protocol.AcquireURI()
u.Update("https://example.com/")
u.Update("/login/redirect_to_sso?redirect=https://example.com/")
fmt.Printf("%q host=%q scheme=%q\n", u.FullURI(), u.Host(), u.Scheme())
// got: "http:///login/redirect_to_sso?redirect=https://example.com/" host="" scheme="http"
// want: "https://example.com/login/redirect_to_sso?redirect=https://example.com/" host="example.com"
```

Control case (same path, no `//` in the query) resolves correctly:

```go
u2 := protocol.AcquireURI()
u2.Update("https://example.com/")
u2.Update("/login/plain")
// "https://example.com/login/plain" host="example.com" ✅
```

End-to-end: `Client.DoRedirects` against any server replying `302` with `Location: /login?redirect=https://example.com/`:

```
err = dial tcp :80: connect: connection refused
req.URI().String() = "http:///login/redirect_to_sso?redirect=https://example.com/"
```

### Expected behavior

Per RFC 3986 reference resolution (and `net/url.URL.ResolveReference`), a new URI should only be treated as absolute when it *starts with* a scheme (`scheme://...`) or is protocol-relative (*starts with* `//`). A leading `/` means path-absolute relative reference; `//` appearing later (e.g. inside the query) must not affect classification. Something like:

```go
if bytes.HasPrefix(newURI, bytestr.StrSlashSlash) || schemePrefixed(newURI) {
// absolute
}
```

(the existing `newURI[0] == '/'` branch already handles the path-absolute case correctly once the misclassification is removed).

### Environment

- hertz v0.10.6 (also reproduces via a downstream fork that forwards to the same implementation)
- Go 1.23, Linux

Happy to send a PR if the proposed direction looks right.

Contributor guide

Open the contributing guide

Research direction

Start in pkg/protocol/uri.go by reading URI.Update, UpdateBytes, and updateBytes, then trace client.DoRequestFollowRedirects through getRedirectURL. Run the minimal reproduction and the control case from the issue. Done means a path-absolute redirect containing an absolute URL in its query preserves the original scheme and host, while absolute and protocol-relative references still resolve correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
networking
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
82/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.