Skip to content

Commit

Permalink
Fix for parsing port at log middleware (#4348)
Browse files Browse the repository at this point in the history
* fix for #4320

Signed-off-by: Srikrishna Paparaju <[email protected]>
Signed-off-by: Srikrishna Paparaju <[email protected]>

* add unit test

Signed-off-by: Srikrishna Paparaju <[email protected]>
  • Loading branch information
spaparaju authored Jun 23, 2021
1 parent 8f50211 commit d8a794b
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 6 deletions.
23 changes: 17 additions & 6 deletions pkg/logging/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"net"
"sort"
"strings"

"net/http"
"time"
Expand Down Expand Up @@ -43,14 +44,24 @@ func (m *HTTPServerMiddleware) HTTPMiddleware(name string, next http.Handler) ht
if hostPort == "" {
hostPort = r.URL.Host
}
_, port, err := net.SplitHostPort(hostPort)
if err != nil {
level.Error(m.logger).Log("msg", "failed to parse host port for http log decision", "err", err)
next.ServeHTTP(w, r)
return

var port string
var err error
// Try to extract port if there is ':' as part of 'hostPort'.
if strings.Contains(hostPort, ":") {
_, port, err = net.SplitHostPort(hostPort)
if err != nil {
level.Error(m.logger).Log("msg", "failed to parse host port for http log decision", "err", err)
next.ServeHTTP(w, r)
return
}
}

decision := m.opts.shouldLog(net.JoinHostPort(r.URL.String(), port), nil)
deciderURL := r.URL.String()
if len(port) > 0 {
deciderURL = net.JoinHostPort(deciderURL, port)
}
decision := m.opts.shouldLog(deciderURL, nil)

switch decision {
case NoLogCall:
Expand Down
15 changes: 15 additions & 0 deletions pkg/logging/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,19 @@ func TestHTTPServerMiddleware(t *testing.T) {
testutil.Equals(t, 200, resp.StatusCode)
testutil.Equals(t, "Test Works", string(body))
testutil.Assert(t, !strings.Contains(b.String(), "err="))

// URL with no explicit port number in the format- hostname:port
req = httptest.NewRequest("GET", "http://example.com/foo", nil)
b.Reset()

w = httptest.NewRecorder()
hm(w, req)

resp = w.Result()
body, err = ioutil.ReadAll(resp.Body)
testutil.Ok(t, err)

testutil.Equals(t, 200, resp.StatusCode)
testutil.Equals(t, "Test Works", string(body))
testutil.Assert(t, !strings.Contains(b.String(), "err="))
}

0 comments on commit d8a794b

Please sign in to comment.