Skip to content

Updated for redirects and skipping unmarshalling #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
djthorpe merged 5 commits into from
Jan 25, 2026
Merged

Updated for redirects and skipping unmarshalling #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
908b095
Updated for redirects
djthorpe Jan 25, 2026
d219ead
Updated based on PR comments
djthorpe Jan 25, 2026
ef78a34
Updated based on comments
djthorpe Jan 25, 2026
ea60459
Updated based on PR comments
djthorpe Jan 25, 2026
5f8eff0
Updated dependencies
djthorpe Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 62 additions & 18 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,25 @@ to a JSON endpoint:
package main

import (
"fmt"
"log"

client "github.com/mutablelogic/go-client"
)

func main() {
// Create a new client
c := client.New(client.OptEndpoint("https://api.example.com/api/v1"))
c, err := client.New(client.OptEndpoint("https://api.example.com/api/v1"))
if err != nil {
log.Fatal(err)
}

// Send a GET request, populating a struct with the response
var response struct {
Message string `json:"message"`
}
if err := c.Do(nil, &response, client.OptPath("test")); err != nil {
// Handle error
log.Fatal(err)
}

// Print the response
Expand All @@ -69,6 +75,16 @@ Various options can be passed to the client `New` method to control its behaviou
* `OptTracer(tracer trace.Tracer)` sets an OpenTelemetry tracer for distributed tracing.
Span names default to "METHOD /path" format. See the OpenTelemetry section below for more details.

## Redirect Handling

The client automatically follows HTTP redirects (3xx responses) for GET and HEAD requests, up to a maximum of 10 redirects. Unlike the default Go HTTP client behavior:

* The HTTP method is preserved (HEAD stays HEAD, GET stays GET)
* Request headers are preserved across redirects
* For security, `Authorization` and `Cookie` headers are stripped when redirecting to a different host

This behavior ensures that redirects work correctly for APIs that use CDNs or load balancers with temporary redirects.

## Usage with a payload

The first argument to the `Do` method is the payload to send to the server, when set.
Expand All @@ -88,24 +104,30 @@ For example,
package main

import (
"fmt"
"log"

client "github.com/mutablelogic/go-client"
)

func main() {
// Create a new client
c := client.New(client.OptEndpoint("https://api.example.com/api/v1"))
c, err := client.New(client.OptEndpoint("https://api.example.com/api/v1"))
if err != nil {
log.Fatal(err)
}

// Send a GET request, populating a struct with the response
// Send a POST request with JSON payload
var request struct {
Prompt string `json:"prompt"`
}
var response struct {
Reply string `json:"reply"`
}
request.Prompt = "Hello, world!"
payload := client.NewJSONRequest(request)
if err := c.Do(payload, &response, OptPath("test")); err != nil {
// Handle error
payload := client.NewJSONRequest(request, client.ContentTypeJson)
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

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

The NewJSONRequest function signature is NewJSONRequest(payload any) (Payload, error) and takes only one parameter. The second parameter client.ContentTypeJson should be removed. If you need to specify the accept type, use NewJSONRequestEx(http.MethodPost, request, client.ContentTypeJson) instead.

Suggested change
payload := client.NewJSONRequest(request, client.ContentTypeJson)
payload := client.NewJSONRequest(request)

Copilot uses AI. Check for mistakes.
if err := c.Do(payload, &response, client.OptPath("test")); err != nil {
log.Fatal(err)
}

// Print the response
Expand Down Expand Up @@ -169,20 +191,27 @@ The authentication token can be set as follows:
package main

import (
"log"
"os"

client "github.com/mutablelogic/go-client"
)

func main() {
// Create a new client
c := client.New(
c, err := client.New(
client.OptEndpoint("https://api.example.com/api/v1"),
client.OptReqToken(client.Token{
Scheme: "Bearer",
Value: os.GetEnv("API_TOKEN"),
Value: os.Getenv("API_TOKEN"),
}),
)
if err != nil {
log.Fatal(err)
}

// ...
// Use the client...
_ = c
}
```

Expand All @@ -203,6 +232,9 @@ The payload should be a `struct` where the fields are converted to form tuples.
package main

import (
"log"
"strings"

client "github.com/mutablelogic/go-client"
multipart "github.com/mutablelogic/go-client/pkg/multipart"
)
Expand All @@ -213,21 +245,27 @@ type FileUpload struct {

func main() {
// Create a new client
c := client.New(client.OptEndpoint("https://api.example.com/api/v1"))
c, err := client.New(client.OptEndpoint("https://api.example.com/api/v1"))
if err != nil {
log.Fatal(err)
}

// Create a file upload request
request := FileUpload{
File: multipart.File{
Path: "helloworld.txt",
Body: strings.NewReader("Hello, world!"),
}
},
}

// Upload a file
if payload, err := client.NewMultipartRequest(request, "*/*"); err != nil {
// Handle error
} else if err := c.Do(payload, &response, OptPath("upload")); err != nil {
// Handle error
var response any
payload, err := client.NewMultipartRequest(request, "*/*")
if err != nil {
log.Fatal(err)
}
if err := c.Do(payload, &response, client.OptPath("upload")); err != nil {
log.Fatal(err)
}
}
```
Expand All @@ -243,7 +281,13 @@ type Unmarshaler interface {
```

The first argument to the `Unmarshal` method is the HTTP header of the response, and the second
argument is the body of the response. The method should return an error if the unmarshalling fails.
argument is the body of the response. You can return one of the following error values from Unmarshal
to indicate how the client should handle the response:

* `nil` to indicate successful unmarshalling.
* `httpresponse.ErrNotImplemented` (from github.com/mutablelogic/go-server/pkg/httpresponse) to fall back to the default unmarshaling behaviour.
In this case, the body will be unmarshaled as JSON, XML, or plain text depending on the Content-Type header.
* Any other error to indicate a failure in unmarshaling.

## Text Streaming Responses

Expand Down Expand Up @@ -444,7 +488,7 @@ This project uses the following third-party libraries:
| [github.com/stretchr/testify](https://pkg.go.dev/github.com/stretchr/testify) | MIT |
| [github.com/andreburgaud/crypt2go](https://pkg.go.dev/github.com/andreburgaud/crypt2go) | BSD-3-Clause |
| [github.com/xdg-go/pbkdf2](https://pkg.go.dev/github.com/xdg-go/pbkdf2) | Apache-2.0 |
| [github.com/djthorpe/go-errors](https://pkg.go.dev/github.com/djthorpe/go-errors) | Apache-2.0 |
| [github.com/mutablelogic/go-server](https://pkg.go.dev/github.com/mutablelogic/go-server) | Apache-2.0 |
| [github.com/djthorpe/go-tablewriter](https://pkg.go.dev/github.com/djthorpe/go-tablewriter) | Apache-2.0 |
| [golang.org/x/crypto](https://pkg.go.dev/golang.org/x/crypto) | BSD-3-Clause |
| [golang.org/x/term](https://pkg.go.dev/golang.org/x/term) | BSD-3-Clause |
Expand Down
117 changes: 95 additions & 22 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@ import (
"time"

// Package imports
pkgotel "github.com/mutablelogic/go-client/pkg/otel"
otel "github.com/mutablelogic/go-client/pkg/otel"
httpresponse "github.com/mutablelogic/go-server/pkg/httpresponse"
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

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

The code imports "github.com/mutablelogic/go-server/pkg/httpresponse" but this dependency is not present in go.mod. This will cause a compilation error. Run "go mod tidy" to add the missing dependency, or add it explicitly with "go get github.com/mutablelogic/go-server".

Suggested change
httpresponse "github.com/mutablelogic/go-server/pkg/httpresponse"

Copilot uses AI. Check for mistakes.
trace "go.opentelemetry.io/otel/trace"

// Namespace imports
. "github.com/djthorpe/go-errors"
)

///////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -97,7 +95,7 @@ func New(opts ...ClientOpt) (*Client, error) {

// If no endpoint, then return error
if this.endpoint == nil {
return nil, ErrBadParameter.With("missing endppint")
return nil, httpresponse.ErrBadRequest.With("missing endpoint")
}

// Return success
Expand All @@ -110,7 +108,7 @@ func New(opts ...ClientOpt) (*Client, error) {
func (client *Client) String() string {
str := "<client"
if client.endpoint != nil {
str += fmt.Sprintf(" endpoint=%q", pkgotel.RedactedURL(client.endpoint))
str += fmt.Sprintf(" endpoint=%q", otel.RedactedURL(client.endpoint))
}
if client.Client.Timeout > 0 {
str += fmt.Sprint(" timeout=", client.Client.Timeout)
Expand Down Expand Up @@ -216,7 +214,7 @@ func (client *Client) Debugf(f string, args ...any) {
func (client *Client) request(ctx context.Context, method, accept, mimetype string, body io.Reader) (*http.Request, error) {
// Return error if no endpoint is set
if client.endpoint == nil {
return nil, ErrBadParameter.With("missing endpoint")
return nil, httpresponse.ErrBadRequest.With("missing endpoint")
}

// Make a request
Expand Down Expand Up @@ -258,6 +256,8 @@ func (client *Client) request(ctx context.Context, method, accept, mimetype stri

// Do will make a JSON request, populate an object with the response and return any errors
func do(client *http.Client, req *http.Request, accept string, strict bool, tracer trace.Tracer, out any, opts ...RequestOpt) (err error) {
const maxRedirects = 10

// Apply request options
reqopts := requestOpts{
Request: req,
Expand All @@ -276,22 +276,78 @@ func do(client *http.Client, req *http.Request, accept string, strict bool, trac
client.Timeout = 0
}

// Create span if tracer provided
// Follow redirects manually so we can keep method and headers for HEAD/GET.
// redirects=0 is the original request, redirects=1..N are redirect follows.
// We allow up to maxRedirects redirect hops (not counting the original request).
var response *http.Response
req, finishSpan := pkgotel.StartHTTPClientSpan(tracer, req)
defer func() { finishSpan(response, err) }()
for redirects := 0; ; redirects++ {
reqWithSpan, finishSpan := otel.StartHTTPClientSpan(tracer, req)
resp, doErr := client.Do(reqWithSpan)
if doErr != nil {
finishSpan(nil, doErr)
return doErr
}

// Do the request
response, err = client.Do(req)
if err != nil {
return err
loc := resp.Header.Get("Location")
isRedirect := resp.StatusCode >= 300 && resp.StatusCode < 400 && loc != ""
canRedirect := req.Method == http.MethodGet || req.Method == http.MethodHead

// Handle redirect responses
if isRedirect {
// Only follow redirects for GET/HEAD methods
if !canRedirect {
resp.Body.Close()
finishSpan(resp, nil)
return httpresponse.Err(resp.StatusCode).Withf("cannot follow redirect for %s request", req.Method)
}

// Check redirect limit: redirects=0 is original, so redirects >= maxRedirects
// means we've already followed maxRedirects hops
if redirects >= maxRedirects {
resp.Body.Close()
finishSpan(resp, nil)
return httpresponse.Err(http.StatusLoopDetected).With("too many redirects")
}

nextURL, parseErr := req.URL.Parse(loc)
if parseErr != nil {
resp.Body.Close()
finishSpan(resp, nil)
return parseErr
}

resp.Body.Close()
finishSpan(resp, nil)

// Clone request for next redirect
nextReq := req.Clone(req.Context())
nextReq.URL = nextURL
nextReq.Host = nextURL.Host

// Strip sensitive headers when redirecting to a different host
// or downgrading from HTTPS to HTTP to prevent credential leakage
crossOrigin := req.URL.Host != nextURL.Host
insecureDowngrade := req.URL.Scheme == "https" && nextURL.Scheme == "http"
if crossOrigin || insecureDowngrade {
nextReq.Header.Del("Authorization")
nextReq.Header.Del("Proxy-Authorization")
nextReq.Header.Del("Cookie")
}

req = nextReq
continue
}

response = resp
defer func() { finishSpan(response, err) }()
break
}
Comment on lines +279 to 344
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

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

This manual redirect handling logic will never execute because the HTTP client doesn't have CheckRedirect set to disable automatic redirects. The default Go HTTP client automatically follows up to 10 redirects. To make this manual redirect logic work, the client initialization in New() needs to set CheckRedirect to return http.ErrUseLastResponse to prevent automatic redirect following.

Copilot uses AI. Check for mistakes.
defer response.Body.Close()

// Get content type
mimetype, err := respContentType(response)
if err != nil {
return ErrUnexpectedResponse.With(mimetype)
return err
}

// Check status code
Expand All @@ -301,13 +357,19 @@ func do(client *http.Client, req *http.Request, accept string, strict bool, trac
if err != nil {
return err
}
return ErrUnexpectedResponse.With(response.Status, ": ", string(data))
if len(data) == 0 {
return httpresponse.Err(response.StatusCode).With(response.Status)
} else {
return httpresponse.Err(response.StatusCode).Withf("%s: %s", response.Status, string(data))
}
}

// When in strict mode, check content type returned is as expected
// When in strict mode, check content type returned is as expected.
// Use 406 Not Acceptable since this is client-side validation that the
// server's response doesn't match our Accept header expectations.
if strict && (accept != "" && accept != ContentTypeAny) {
if mimetype != accept {
return ErrUnexpectedResponse.Withf("strict mode: unexpected response with %q", mimetype)
return httpresponse.Err(http.StatusNotAcceptable).Withf("strict mode: expected %q, got %q", accept, mimetype)
}
Comment on lines +367 to 373
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

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

Using http.StatusNotAcceptable (406) for client-side validation errors may be misleading. The actual HTTP response was successful (2xx), but the error suggests the server returned a 406 status. Consider using a different error type that doesn't imply an HTTP status code from the server, such as a custom validation error or ErrBadRequest, to avoid confusion.

Copilot uses AI. Check for mistakes.
}

Expand All @@ -316,9 +378,20 @@ func do(client *http.Client, req *http.Request, accept string, strict bool, trac
return nil
}

// Decode the body, preferring custom Unmarshaler when implemented
// Decode the body, preferring custom Unmarshaler when implemented. If the Unmarshaler
// returns httpresponse.ErrNotImplemented, then fall through to default unmarshaling
if v, ok := out.(Unmarshaler); ok {
return v.Unmarshal(response.Header, response.Body)
if err := v.Unmarshal(response.Header, response.Body); err != nil {
var httpErr httpresponse.Err
if errors.As(err, &httpErr) && int(httpErr) == http.StatusNotImplemented {
// Fall through to default unmarshaling
} else {
return err
}
} else {
// Unmarshaling successful
return nil
}
}
Comment on lines +381 to 395
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

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

If the Unmarshaler reads from the response.Body before returning ErrNotImplemented, the body will be partially or fully consumed, and the fallback to default unmarshaling will fail. Consider documenting that Unmarshaler implementations must return ErrNotImplemented without reading from the body if they want to fall back to default unmarshaling, or use a buffer/seeker to allow re-reading the body.

Copilot uses AI. Check for mistakes.

switch mimetype {
Expand Down Expand Up @@ -352,7 +425,7 @@ func do(client *http.Client, req *http.Request, accept string, strict bool, trac
return err
}
} else {
return ErrInternalAppError.Withf("do: response does not implement Unmarshaler for %q", mimetype)
return httpresponse.ErrInternalError.Withf("do: response does not implement Unmarshaler for %q", mimetype)
}
}

Expand All @@ -367,7 +440,7 @@ func respContentType(resp *http.Response) (string, error) {
return ContentTypeBinary, nil
}
if mimetype, _, err := mime.ParseMediaType(contenttype); err != nil {
return contenttype, ErrUnexpectedResponse.With(contenttype)
return contenttype, httpresponse.Err(http.StatusUnsupportedMediaType).With(contenttype)
} else {
return mimetype, nil
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/andreburgaud/crypt2go v1.8.0
github.com/djthorpe/go-errors v1.0.3
github.com/djthorpe/go-tablewriter v0.0.11
github.com/mutablelogic/go-server v1.5.18
github.com/stretchr/testify v1.11.1
github.com/xdg-go/pbkdf2 v1.0.0
go.opentelemetry.io/otel v1.39.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/mattn/go-runewidth v0.0.19 h1:v++JhqYnZuu5jSKrk9RbgF5v4CGUjqRfBm05byFGLdw=
github.com/mattn/go-runewidth v0.0.19/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs=
github.com/mutablelogic/go-server v1.5.18 h1:UKpJQReabHFMz1U/gbOq/+Q0C0ZzklVBalvs5FFk9NQ=
github.com/mutablelogic/go-server v1.5.18/go.mod h1:swZf3T0eGe9VEE0Ki37WknpN+XxTGj2Xn6EP7BJm9x0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ=
Expand Down
Loading
Loading