Skip to content
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.

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

http: Add the ability to not follow redirects #278

Merged
merged 4 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
18 changes: 17 additions & 1 deletion config/http_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ import (
"gopkg.in/yaml.v2"
)

// DefaultHTTPClientConfig is the default HTTP client configuration.
var DefaultHTTPClientConfig = HTTPClientConfig{
FollowRedirects: true,
}

type closeIdler interface {
CloseIdleConnections()
}
Expand Down Expand Up @@ -111,6 +116,10 @@ type HTTPClientConfig struct {
ProxyURL URL `yaml:"proxy_url,omitempty"`
// TLSConfig to use to connect to the targets.
TLSConfig TLSConfig `yaml:"tls_config,omitempty"`
// FollowRedirects specifies wheter the client should follow HTTP 3xx redirects.
Copy link
Member

Choose a reason for hiding this comment

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

Still "wheter" → "whether"

// The omitempty flag is not set, because it would be hidden from the
// marshalled configuration when set to false.
FollowRedirects bool `yaml:"follow_redirects"`
}

// SetDirectory joins any relative file paths with dir.
Expand Down Expand Up @@ -172,6 +181,7 @@ func (c *HTTPClientConfig) Validate() error {
// UnmarshalYAML implements the yaml.Unmarshaler interface
func (c *HTTPClientConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
type plain HTTPClientConfig
*c = DefaultHTTPClientConfig
Copy link
Member

Choose a reason for hiding this comment

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

As said, this wouldn't work if c were nil. (Which implies that c is not nil rather than that we are doing anything wrong here.)

if err := unmarshal((*plain)(c)); err != nil {
return err
}
Expand All @@ -196,7 +206,13 @@ func NewClientFromConfig(cfg HTTPClientConfig, name string, disableKeepAlives, e
if err != nil {
return nil, err
}
return newClient(rt), nil
client := newClient(rt)
if !cfg.FollowRedirects {
client.CheckRedirect = func(*http.Request, []*http.Request) error {
return http.ErrUseLastResponse
}
}
return client, nil
}

// NewRoundTripperFromConfig returns a new HTTP RoundTripper configured for the
Expand Down
52 changes: 51 additions & 1 deletion config/http_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,46 @@ func TestNewClientFromConfig(t *testing.T) {
fmt.Fprint(w, ExpectedMessage)
}
},
}, {
clientConfig: HTTPClientConfig{
FollowRedirects: true,
TLSConfig: TLSConfig{
CAFile: TLSCAChainPath,
CertFile: ClientCertificatePath,
KeyFile: ClientKeyNoPassPath,
ServerName: "",
InsecureSkipVerify: false},
},
handler: func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/redirected":
fmt.Fprintf(w, ExpectedMessage)
default:
w.Header().Set("Location", "/redirected")
w.WriteHeader(http.StatusFound)
fmt.Fprintf(w, "It should follow the redirect.")
}
},
}, {
clientConfig: HTTPClientConfig{
FollowRedirects: false,
TLSConfig: TLSConfig{
CAFile: TLSCAChainPath,
CertFile: ClientCertificatePath,
KeyFile: ClientKeyNoPassPath,
ServerName: "",
InsecureSkipVerify: false},
},
handler: func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/redirected":
fmt.Fprint(w, "The redirection was followed.")
default:
w.Header().Set("Location", "/redirected")
w.WriteHeader(http.StatusFound)
fmt.Fprintf(w, ExpectedMessage)
}
},
},
}

Expand All @@ -317,7 +357,7 @@ func TestNewClientFromConfig(t *testing.T) {
}
response, err := client.Get(testServer.URL)
if err != nil {
t.Errorf("Can't connect to the test server using this config: %+v", validConfig.clientConfig)
t.Errorf("Can't connect to the test server using this config: %+v %v", validConfig.clientConfig, err)
Copy link
Member

Choose a reason for hiding this comment

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

It's kind of hard to see that there is an error following after the config. How about:

"Can't connect to the test server using config %+v: %v"

continue
}

Expand Down Expand Up @@ -932,6 +972,16 @@ func TestHideHTTPClientConfigSecrets(t *testing.T) {
}
}

func TestDefaultFollowRedirect(t *testing.T) {
cfg, _, err := LoadHTTPConfigFile("testdata/http.conf.good.yml")
if err != nil {
t.Errorf("Error loading HTTP client config: %v", err)
}
if !cfg.FollowRedirects {
t.Errorf("follow_redirects should be true")
}
}

func TestValidateHTTPConfig(t *testing.T) {
cfg, _, err := LoadHTTPConfigFile("testdata/http.conf.good.yml")
if err != nil {
Expand Down