From 318cd413fc0b69780c0c513d2dc175fa8bf5cc31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Tue, 18 Feb 2020 15:52:29 +0100 Subject: [PATCH 1/4] Don't return error in ContextFromRequest function. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously it could return error if RemoteAddr didn't have correct format, but since this field has no specified format, that was little too strict. Signed-off-by: Peter Štibraný --- util/httputil/context.go | 24 ++++++++++++++---------- web/api/v1/api.go | 10 ++-------- web/web.go | 6 +----- 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/util/httputil/context.go b/util/httputil/context.go index b32fb9c576..5b0b983cb2 100644 --- a/util/httputil/context.go +++ b/util/httputil/context.go @@ -33,20 +33,24 @@ func ContextWithPath(ctx context.Context, path string) context.Context { // ContextFromRequest returns a new context from a requests with identifiers of // the request to be used later when logging the query. -func ContextFromRequest(ctx context.Context, r *http.Request) (context.Context, error) { - ip, _, err := net.SplitHostPort(r.RemoteAddr) - if err != nil { - return ctx, err +func ContextFromRequest(ctx context.Context, r *http.Request) context.Context { + m := map[string]string{ + "method": r.Method, } + + // r.RemoteAddr has no defined format, so don't return error if we cannot split it into IP:Port. + ip, _, _ := net.SplitHostPort(r.RemoteAddr) + if ip != "" { + m["clientIP"] = ip + } + var path string if v := ctx.Value(pathParam); v != nil { path = v.(string) + m["path"] = path } + return promql.NewOriginContext(ctx, map[string]interface{}{ - "httpRequest": map[string]string{ - "clientIP": ip, - "method": r.Method, - "path": path, - }, - }), nil + "httpRequest": m, + }) } diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 32cc8fa758..0baee0823b 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -348,10 +348,7 @@ func (api *API) query(r *http.Request) apiFuncResult { return apiFuncResult{nil, &apiError{errorBadData, err}, nil, nil} } - ctx, err = httputil.ContextFromRequest(ctx, r) - if err != nil { - return apiFuncResult{nil, returnAPIError(err), nil, nil} - } + ctx = httputil.ContextFromRequest(ctx, r) res := qry.Exec(ctx) if res.Err != nil { @@ -423,10 +420,7 @@ func (api *API) queryRange(r *http.Request) apiFuncResult { return apiFuncResult{nil, &apiError{errorBadData, err}, nil, nil} } - ctx, err = httputil.ContextFromRequest(ctx, r) - if err != nil { - return apiFuncResult{nil, returnAPIError(err), nil, nil} - } + ctx = httputil.ContextFromRequest(ctx, r) res := qry.Exec(ctx) if res.Err != nil { diff --git a/web/web.go b/web/web.go index e25878928c..21fc02158a 100644 --- a/web/web.go +++ b/web/web.go @@ -653,11 +653,7 @@ func (h *Handler) consoles(w http.ResponseWriter, r *http.Request) { return } - ctx, err = httputil.ContextFromRequest(ctx, r) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } + ctx = httputil.ContextFromRequest(ctx, r) // Provide URL parameters as a map for easy use. Advanced users may have need for // parameters beyond the first, so provide RawParams. From baa6f60384628c8ad9b8ce1dd30468c8a390d0fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Tue, 18 Feb 2020 16:22:26 +0100 Subject: [PATCH 2/4] Changed var name. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- util/httputil/context.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/util/httputil/context.go b/util/httputil/context.go index 5b0b983cb2..10169d8901 100644 --- a/util/httputil/context.go +++ b/util/httputil/context.go @@ -34,23 +34,23 @@ func ContextWithPath(ctx context.Context, path string) context.Context { // ContextFromRequest returns a new context from a requests with identifiers of // the request to be used later when logging the query. func ContextFromRequest(ctx context.Context, r *http.Request) context.Context { - m := map[string]string{ + reqCtxVal := map[string]string{ "method": r.Method, } // r.RemoteAddr has no defined format, so don't return error if we cannot split it into IP:Port. ip, _, _ := net.SplitHostPort(r.RemoteAddr) if ip != "" { - m["clientIP"] = ip + reqCtxVal["clientIP"] = ip } var path string if v := ctx.Value(pathParam); v != nil { path = v.(string) - m["path"] = path + reqCtxVal["path"] = path } return promql.NewOriginContext(ctx, map[string]interface{}{ - "httpRequest": m, + "httpRequest": reqCtxVal, }) } From fe3fe5740ed3ce7c4aedb6fd084d959daae9b7d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Tue, 18 Feb 2020 17:35:16 +0100 Subject: [PATCH 3/4] Updated comment. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- util/httputil/context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/httputil/context.go b/util/httputil/context.go index 10169d8901..adf93dd0a5 100644 --- a/util/httputil/context.go +++ b/util/httputil/context.go @@ -31,7 +31,7 @@ func ContextWithPath(ctx context.Context, path string) context.Context { return context.WithValue(ctx, pathParam, path) } -// ContextFromRequest returns a new context from a requests with identifiers of +// ContextFromRequest returns a new context with identifiers of // the request to be used later when logging the query. func ContextFromRequest(ctx context.Context, r *http.Request) context.Context { reqCtxVal := map[string]string{ From a7d3a456d67624472f8d6a846ed41024064526d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Wed, 19 Feb 2020 11:57:20 +0100 Subject: [PATCH 4/4] =?UTF-8?q?Updated=20code=20based=20on=20Juliens=20com?= =?UTF-8?q?ments=20(via=20Slack).=20Signed-off-by:=20Peter=20S=CC=8Ctibran?= =?UTF-8?q?y=CC=81=20?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- util/httputil/context.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/util/httputil/context.go b/util/httputil/context.go index adf93dd0a5..89bdac6425 100644 --- a/util/httputil/context.go +++ b/util/httputil/context.go @@ -34,23 +34,20 @@ func ContextWithPath(ctx context.Context, path string) context.Context { // ContextFromRequest returns a new context with identifiers of // the request to be used later when logging the query. func ContextFromRequest(ctx context.Context, r *http.Request) context.Context { - reqCtxVal := map[string]string{ - "method": r.Method, + var ip string + if r.RemoteAddr != "" { + // r.RemoteAddr has no defined format, so don't return error if we cannot split it into IP:Port. + ip, _, _ = net.SplitHostPort(r.RemoteAddr) } - - // r.RemoteAddr has no defined format, so don't return error if we cannot split it into IP:Port. - ip, _, _ := net.SplitHostPort(r.RemoteAddr) - if ip != "" { - reqCtxVal["clientIP"] = ip - } - var path string if v := ctx.Value(pathParam); v != nil { path = v.(string) - reqCtxVal["path"] = path } - return promql.NewOriginContext(ctx, map[string]interface{}{ - "httpRequest": reqCtxVal, + "httpRequest": map[string]string{ + "clientIP": ip, + "method": r.Method, + "path": path, + }, }) }