From cf9b9802a8732424b6cb7c233e3bfcf8cb3530ee Mon Sep 17 00:00:00 2001 From: Andreas Linz Date: Wed, 3 Oct 2018 19:28:44 +0200 Subject: Set a proper HTTP user-agent header (#9482) Previously, mattermost-server would always request with the default user-agent of Go's net/http package that is `Go-http-client/1.1` or something similar. This has several disadvantages, one is that the default user-agent made it pretty hard to distinguish mattermost requests from other service requests in a network log for example. Now a user-agent of the form `mattermost-` is set in the client. - [x] Added or updated unit tests (required for all new features) --- app/post.go | 6 ++++-- services/httpservice/client.go | 18 ++++++++++++++++-- services/httpservice/client_test.go | 21 +++++++++++++++++++++ services/httpservice/httpservice.go | 5 ++--- utils/testutils/mocked_http_service.go | 5 +++-- 5 files changed, 46 insertions(+), 9 deletions(-) diff --git a/app/post.go b/app/post.go index 474862b66..fe726e343 100644 --- a/app/post.go +++ b/app/post.go @@ -16,12 +16,14 @@ import ( "strings" "github.com/dyatlov/go-opengraph/opengraph" + "golang.org/x/net/html/charset" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" + "github.com/mattermost/mattermost-server/services/httpservice" "github.com/mattermost/mattermost-server/store" "github.com/mattermost/mattermost-server/utils" - "golang.org/x/net/html/charset" ) func (a *App) CreatePostAsUser(post *model.Post, clearPushNotifications bool) (*model.Post, *model.AppError) { @@ -919,7 +921,7 @@ func (a *App) DoPostAction(postId, actionId, userId, selectedOption string) *mod req.Header.Set("Accept", "application/json") // Allow access to plugin routes for action buttons - var httpClient *http.Client + var httpClient *httpservice.Client url, _ := url.Parse(action.Integration.URL) siteURL, _ := url.Parse(*a.Config().ServiceSettings.SiteURL) subpath, _ := utils.GetSubpathFromConfig(a.Config()) diff --git a/services/httpservice/client.go b/services/httpservice/client.go index 268f63b24..1e7b7b5f9 100644 --- a/services/httpservice/client.go +++ b/services/httpservice/client.go @@ -10,6 +10,8 @@ import ( "net" "net/http" "time" + + "github.com/mattermost/mattermost-server/model" ) const ( @@ -28,6 +30,8 @@ func IsReservedIP(ip net.IP) bool { return false } +var defaultUserAgent string + func init() { for _, cidr := range []string{ // See https://tools.ietf.org/html/rfc6890 @@ -48,6 +52,7 @@ func init() { } reservedIPRanges = append(reservedIPRanges, parsed) } + defaultUserAgent = "mattermost-" + model.CurrentVersion } type DialContextFunction func(ctx context.Context, network, addr string) (net.Conn, error) @@ -97,6 +102,15 @@ func dialContextFilter(dial DialContextFunction, allowHost func(host string) boo } } +type Client struct { + *http.Client +} + +func (c *Client) Do(req *http.Request) (*http.Response, error) { + req.Header.Set("User-Agent", defaultUserAgent) + return c.Client.Do(req) +} + // NewHTTPClient returns a variation the default implementation of Client. // It uses a Transport with the same settings as the default Transport // but with the following modifications: @@ -104,7 +118,7 @@ func dialContextFilter(dial DialContextFunction, allowHost func(host string) boo // "connectTimeout") // - timeout for the end-to-end request (defined as constant // "requestTimeout") -func NewHTTPClient(enableInsecureConnections bool, allowHost func(host string) bool, allowIP func(ip net.IP) bool) *http.Client { +func NewHTTPClient(enableInsecureConnections bool, allowHost func(host string) bool, allowIP func(ip net.IP) bool) *Client { dialContext := (&net.Dialer{ Timeout: connectTimeout, KeepAlive: 30 * time.Second, @@ -129,5 +143,5 @@ func NewHTTPClient(enableInsecureConnections bool, allowHost func(host string) b Timeout: requestTimeout, } - return client + return &Client{Client: client} } diff --git a/services/httpservice/client_test.go b/services/httpservice/client_test.go index ceb133140..174ddf2de 100644 --- a/services/httpservice/client_test.go +++ b/services/httpservice/client_test.go @@ -118,3 +118,24 @@ func TestDialContextFilter(t *testing.T) { } } } + +func TestUserAgentIsSet(t *testing.T) { + testUserAgent := "test-user-agent" + defaultUserAgent = testUserAgent + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + ua := req.UserAgent() + if ua == "" { + t.Error("expected user-agent to be non-empty") + } + if ua != testUserAgent { + t.Errorf("expected user-agent to be %q but was %q", testUserAgent, ua) + } + })) + defer ts.Close() + client := NewHTTPClient(true, nil, nil) + req, err := http.NewRequest("GET", ts.URL, nil) + if err != nil { + t.Fatal("NewRequest failed", err) + } + client.Do(req) +} diff --git a/services/httpservice/httpservice.go b/services/httpservice/httpservice.go index 5ed42a12d..6e776890f 100644 --- a/services/httpservice/httpservice.go +++ b/services/httpservice/httpservice.go @@ -5,7 +5,6 @@ package httpservice import ( "net" - "net/http" "strings" "github.com/mattermost/mattermost-server/services/configservice" @@ -13,7 +12,7 @@ import ( // Wraps the functionality for creating a new http.Client to encapsulate that and allow it to be mocked when testing type HTTPService interface { - MakeClient(trustURLs bool) *http.Client + MakeClient(trustURLs bool) *Client Close() } @@ -25,7 +24,7 @@ func MakeHTTPService(configService configservice.ConfigService) HTTPService { return &HTTPServiceImpl{configService} } -func (h *HTTPServiceImpl) MakeClient(trustURLs bool) *http.Client { +func (h *HTTPServiceImpl) MakeClient(trustURLs bool) *Client { insecure := h.configService.Config().ServiceSettings.EnableInsecureOutgoingConnections != nil && *h.configService.Config().ServiceSettings.EnableInsecureOutgoingConnections if trustURLs { diff --git a/utils/testutils/mocked_http_service.go b/utils/testutils/mocked_http_service.go index b1e7f6963..8f9bc42be 100644 --- a/utils/testutils/mocked_http_service.go +++ b/utils/testutils/mocked_http_service.go @@ -4,6 +4,7 @@ package testutils import ( + "github.com/mattermost/mattermost-server/services/httpservice" "net/http" "net/http/httptest" ) @@ -18,8 +19,8 @@ func MakeMockedHTTPService(handler http.Handler) *MockedHTTPService { } } -func (h *MockedHTTPService) MakeClient(trustURLs bool) *http.Client { - return h.Server.Client() +func (h *MockedHTTPService) MakeClient(trustURLs bool) *httpservice.Client { + return &httpservice.Client{Client: h.Server.Client()} } func (h *MockedHTTPService) Close() { -- cgit v1.2.3-1-g7c22