Commit 0a2d5e16 by Arve Knudsen Committed by GitHub

pkg/api: Check errors (#19657)

* pkg/api: Check errors
* pkg/api: Remove unused function HashEmail
parent dabc848e
...@@ -9,8 +9,6 @@ package avatar ...@@ -9,8 +9,6 @@ package avatar
import ( import (
"bufio" "bufio"
"bytes" "bytes"
"crypto/md5"
"encoding/hex"
"fmt" "fmt"
"io" "io"
"io/ioutil" "io/ioutil"
...@@ -43,18 +41,6 @@ func UpdateGravatarSource() { ...@@ -43,18 +41,6 @@ func UpdateGravatarSource() {
} }
} }
// hash email to md5 string
// keep this func in order to make this package independent
func HashEmail(email string) string {
// https://en.gravatar.com/site/implement/hash/
email = strings.TrimSpace(email)
email = strings.ToLower(email)
h := md5.New()
h.Write([]byte(email))
return hex.EncodeToString(h.Sum(nil))
}
// Avatar represents the avatar object. // Avatar represents the avatar object.
type Avatar struct { type Avatar struct {
hash string hash string
...@@ -119,7 +105,9 @@ func (this *CacheServer) Handler(ctx *macaron.Context) { ...@@ -119,7 +105,9 @@ func (this *CacheServer) Handler(ctx *macaron.Context) {
if avatar.notFound { if avatar.notFound {
avatar = this.notFound avatar = this.notFound
} else { } else {
this.cache.Add(hash, avatar, gocache.DefaultExpiration) if err := this.cache.Add(hash, avatar, gocache.DefaultExpiration); err != nil {
log.Warn("Error adding avatar to cache: %s", err)
}
} }
ctx.Resp.Header().Add("Content-Type", "image/jpeg") ctx.Resp.Header().Add("Content-Type", "image/jpeg")
......
...@@ -55,7 +55,9 @@ func (r *NormalResponse) WriteTo(ctx *m.ReqContext) { ...@@ -55,7 +55,9 @@ func (r *NormalResponse) WriteTo(ctx *m.ReqContext) {
header[k] = v header[k] = v
} }
ctx.Resp.WriteHeader(r.status) ctx.Resp.WriteHeader(r.status)
ctx.Resp.Write(r.body) if _, err := ctx.Resp.Write(r.body); err != nil {
ctx.Logger.Error("Error writing to response", "err", err)
}
} }
func (r *NormalResponse) Cache(ttl string) *NormalResponse { func (r *NormalResponse) Cache(ttl string) *NormalResponse {
......
...@@ -152,9 +152,10 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) { ...@@ -152,9 +152,10 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) {
mockSnapshotResult.UserId = TestUserID mockSnapshotResult.UserId = TestUserID
Convey("Should gracefully delete local snapshot when remote snapshot has already been removed", func() { Convey("Should gracefully delete local snapshot when remote snapshot has already been removed", func() {
var writeErr error
loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) { loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) { ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
rw.Write([]byte(`{"message":"Failed to get dashboard snapshot"}`)) _, writeErr = rw.Write([]byte(`{"message":"Failed to get dashboard snapshot"}`))
rw.WriteHeader(500) rw.WriteHeader(500)
}) })
...@@ -162,21 +163,24 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) { ...@@ -162,21 +163,24 @@ func TestDashboardSnapshotApiEndpoint(t *testing.T) {
sc.handlerFunc = DeleteDashboardSnapshot sc.handlerFunc = DeleteDashboardSnapshot
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec() sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
So(writeErr, ShouldBeNil)
So(sc.resp.Code, ShouldEqual, 200) So(sc.resp.Code, ShouldEqual, 200)
}) })
}) })
Convey("Should fail to delete local snapshot when an unexpected 500 error occurs", func() { Convey("Should fail to delete local snapshot when an unexpected 500 error occurs", func() {
loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) { loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/snapshots/12345", "/api/snapshots/:key", m.ROLE_EDITOR, func(sc *scenarioContext) {
var writeErr error
ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) { ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
rw.WriteHeader(500) rw.WriteHeader(500)
rw.Write([]byte(`{"message":"Unexpected"}`)) _, writeErr = rw.Write([]byte(`{"message":"Unexpected"}`))
}) })
mockSnapshotResult.ExternalDeleteUrl = ts.URL mockSnapshotResult.ExternalDeleteUrl = ts.URL
sc.handlerFunc = DeleteDashboardSnapshot sc.handlerFunc = DeleteDashboardSnapshot
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec() sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
So(writeErr, ShouldBeNil)
So(sc.resp.Code, ShouldEqual, 500) So(sc.resp.Code, ShouldEqual, 500)
}) })
}) })
......
...@@ -7,6 +7,7 @@ import ( ...@@ -7,6 +7,7 @@ import (
"strings" "strings"
"github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/infra/log"
m "github.com/grafana/grafana/pkg/models" m "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
) )
...@@ -61,7 +62,9 @@ func GetGravatarUrl(text string) string { ...@@ -61,7 +62,9 @@ func GetGravatarUrl(text string) string {
} }
hasher := md5.New() hasher := md5.New()
hasher.Write([]byte(strings.ToLower(text))) if _, err := hasher.Write([]byte(strings.ToLower(text))); err != nil {
log.Warn("Failed to hash text: %s", err)
}
return fmt.Sprintf(setting.AppSubUrl+"/avatar/%x", hasher.Sum(nil)) return fmt.Sprintf(setting.AppSubUrl+"/avatar/%x", hasher.Sum(nil))
} }
......
...@@ -125,16 +125,19 @@ func (hs *HTTPServer) Run(ctx context.Context) error { ...@@ -125,16 +125,19 @@ func (hs *HTTPServer) Run(ctx context.Context) error {
case setting.SOCKET: case setting.SOCKET:
ln, err := net.ListenUnix("unix", &net.UnixAddr{Name: setting.SocketPath, Net: "unix"}) ln, err := net.ListenUnix("unix", &net.UnixAddr{Name: setting.SocketPath, Net: "unix"})
if err != nil { if err != nil {
hs.log.Debug("server was shutdown gracefully") hs.log.Debug("server was shutdown gracefully", "err", err)
return nil return nil
} }
// Make socket writable by group // Make socket writable by group
os.Chmod(setting.SocketPath, 0660) if err := os.Chmod(setting.SocketPath, 0660); err != nil {
hs.log.Debug("server was shutdown gracefully", "err", err)
return nil
}
err = hs.httpSrv.Serve(ln) err = hs.httpSrv.Serve(ln)
if err != nil { if err != nil {
hs.log.Debug("server was shutdown gracefully") hs.log.Debug("server was shutdown gracefully", "err", err)
return nil return nil
} }
default: default:
...@@ -340,7 +343,9 @@ func (hs *HTTPServer) healthHandler(ctx *macaron.Context) { ...@@ -340,7 +343,9 @@ func (hs *HTTPServer) healthHandler(ctx *macaron.Context) {
} }
dataBytes, _ := data.EncodePretty() dataBytes, _ := data.EncodePretty()
ctx.Resp.Write(dataBytes) if _, err := ctx.Resp.Write(dataBytes); err != nil {
hs.log.Error("Failed to write to response", "err", err)
}
} }
func (hs *HTTPServer) mapStatic(m *macaron.Macaron, rootDir string, dir string, prefix string) { func (hs *HTTPServer) mapStatic(m *macaron.Macaron, rootDir string, dir string, prefix string) {
......
...@@ -35,13 +35,15 @@ type connection struct { ...@@ -35,13 +35,15 @@ type connection struct {
hub *hub hub *hub
ws *websocket.Conn ws *websocket.Conn
send chan []byte send chan []byte
log log.Logger
} }
func newConnection(ws *websocket.Conn, hub *hub) *connection { func newConnection(ws *websocket.Conn, hub *hub, logger log.Logger) *connection {
return &connection{ return &connection{
hub: hub, hub: hub,
send: make(chan []byte, 256), send: make(chan []byte, 256),
ws: ws, ws: ws,
log: logger,
} }
} }
...@@ -52,13 +54,17 @@ func (c *connection) readPump() { ...@@ -52,13 +54,17 @@ func (c *connection) readPump() {
}() }()
c.ws.SetReadLimit(maxMessageSize) c.ws.SetReadLimit(maxMessageSize)
c.ws.SetReadDeadline(time.Now().Add(pongWait)) if err := c.ws.SetReadDeadline(time.Now().Add(pongWait)); err != nil {
c.ws.SetPongHandler(func(string) error { c.ws.SetReadDeadline(time.Now().Add(pongWait)); return nil }) c.log.Warn("Setting read deadline failed", "err", err)
}
c.ws.SetPongHandler(func(string) error {
return c.ws.SetReadDeadline(time.Now().Add(pongWait))
})
for { for {
_, message, err := c.ws.ReadMessage() _, message, err := c.ws.ReadMessage()
if err != nil { if err != nil {
if websocket.IsUnexpectedCloseError(err, websocket.CloseGoingAway) { if websocket.IsUnexpectedCloseError(err, websocket.CloseGoingAway) {
log.Info("error: %v", err) c.log.Info("error", "err", err)
} }
break break
} }
...@@ -91,7 +97,9 @@ func (c *connection) handleMessage(message []byte) { ...@@ -91,7 +97,9 @@ func (c *connection) handleMessage(message []byte) {
} }
func (c *connection) write(mt int, payload []byte) error { func (c *connection) write(mt int, payload []byte) error {
c.ws.SetWriteDeadline(time.Now().Add(writeWait)) if err := c.ws.SetWriteDeadline(time.Now().Add(writeWait)); err != nil {
return err
}
return c.ws.WriteMessage(mt, payload) return c.ws.WriteMessage(mt, payload)
} }
...@@ -106,7 +114,9 @@ func (c *connection) writePump() { ...@@ -106,7 +114,9 @@ func (c *connection) writePump() {
select { select {
case message, ok := <-c.send: case message, ok := <-c.send:
if !ok { if !ok {
c.write(websocket.CloseMessage, []byte{}) if err := c.write(websocket.CloseMessage, []byte{}); err != nil {
c.log.Warn("Failed to write close message to connection", "err", err)
}
return return
} }
if err := c.write(websocket.TextMessage, message); err != nil { if err := c.write(websocket.TextMessage, message); err != nil {
......
...@@ -44,7 +44,7 @@ func (sm *StreamManager) Serve(w http.ResponseWriter, r *http.Request) { ...@@ -44,7 +44,7 @@ func (sm *StreamManager) Serve(w http.ResponseWriter, r *http.Request) {
return return
} }
c := newConnection(ws, sm.hub) c := newConnection(ws, sm.hub, sm.log)
sm.hub.register <- c sm.hub.register <- c
go c.writePump() go c.writePump()
......
...@@ -29,10 +29,13 @@ var ( ...@@ -29,10 +29,13 @@ var (
OauthStateCookieName = "oauth_state" OauthStateCookieName = "oauth_state"
) )
func GenStateString() string { func GenStateString() (string, error) {
rnd := make([]byte, 32) rnd := make([]byte, 32)
rand.Read(rnd) if _, err := rand.Read(rnd); err != nil {
return base64.URLEncoding.EncodeToString(rnd) oauthLogger.Error("failed to generate state string", "err", err)
return "", err
}
return base64.URLEncoding.EncodeToString(rnd), nil
} }
func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) {
...@@ -58,7 +61,13 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) { ...@@ -58,7 +61,13 @@ func (hs *HTTPServer) OAuthLogin(ctx *m.ReqContext) {
code := ctx.Query("code") code := ctx.Query("code")
if code == "" { if code == "" {
state := GenStateString() state, err := GenStateString()
if err != nil {
ctx.Logger.Error("Generating state string failed", "err", err)
ctx.Handle(500, "An internal error occurred", nil)
return
}
hashedState := hashStatecode(state, setting.OAuthService.OAuthInfos[name].ClientSecret) hashedState := hashStatecode(state, setting.OAuthService.OAuthInfos[name].ClientSecret)
hs.writeCookie(ctx.Resp, OauthStateCookieName, hashedState, 60, hs.Cfg.CookieSameSite) hs.writeCookie(ctx.Resp, OauthStateCookieName, hashedState, 60, hs.Cfg.CookieSameSite)
if setting.OAuthService.OAuthInfos[name].HostedDomain == "" { if setting.OAuthService.OAuthInfos[name].HostedDomain == "" {
...@@ -239,7 +248,9 @@ func hashStatecode(code, seed string) string { ...@@ -239,7 +248,9 @@ func hashStatecode(code, seed string) string {
func (hs *HTTPServer) redirectWithError(ctx *m.ReqContext, err error, v ...interface{}) { func (hs *HTTPServer) redirectWithError(ctx *m.ReqContext, err error, v ...interface{}) {
ctx.Logger.Error(err.Error(), v...) ctx.Logger.Error(err.Error(), v...)
hs.trySetEncryptedCookie(ctx, LoginErrorCookieName, err.Error(), 60) if err := hs.trySetEncryptedCookie(ctx, LoginErrorCookieName, err.Error(), 60); err != nil {
oauthLogger.Error("Failed to set encrypted cookie", "err", err)
}
ctx.Redirect(setting.AppSubUrl + "/login") ctx.Redirect(setting.AppSubUrl + "/login")
} }
...@@ -177,10 +177,12 @@ func (hs *HTTPServer) CompleteInvite(c *m.ReqContext, completeInvite dtos.Comple ...@@ -177,10 +177,12 @@ func (hs *HTTPServer) CompleteInvite(c *m.ReqContext, completeInvite dtos.Comple
user := &cmd.Result user := &cmd.Result
bus.Publish(&events.SignUpCompleted{ if err := bus.Publish(&events.SignUpCompleted{
Name: user.NameOrFallback(), Name: user.NameOrFallback(),
Email: user.Email, Email: user.Email,
}) }); err != nil {
return Error(500, "failed to publish event", err)
}
if ok, rsp := applyUserInvite(user, invite, true); !ok { if ok, rsp := applyUserInvite(user, invite, true); !ok {
return rsp return rsp
......
...@@ -96,10 +96,12 @@ func (proxy *DataSourceProxy) HandleRequest() { ...@@ -96,10 +96,12 @@ func (proxy *DataSourceProxy) HandleRequest() {
proxy.addTraceFromHeaderValue(span, "X-Panel-Id", "panel_id") proxy.addTraceFromHeaderValue(span, "X-Panel-Id", "panel_id")
proxy.addTraceFromHeaderValue(span, "X-Dashboard-Id", "dashboard_id") proxy.addTraceFromHeaderValue(span, "X-Dashboard-Id", "dashboard_id")
opentracing.GlobalTracer().Inject( if err := opentracing.GlobalTracer().Inject(
span.Context(), span.Context(),
opentracing.HTTPHeaders, opentracing.HTTPHeaders,
opentracing.HTTPHeadersCarrier(proxy.ctx.Req.Request.Header)) opentracing.HTTPHeadersCarrier(proxy.ctx.Req.Request.Header)); err != nil {
logger.Error("Failed to inject span context instance", "err", err)
}
originalSetCookie := proxy.ctx.Resp.Header().Get("Set-Cookie") originalSetCookie := proxy.ctx.Resp.Header().Get("Set-Cookie")
......
...@@ -500,10 +500,11 @@ func TestDSRouteRule(t *testing.T) { ...@@ -500,10 +500,11 @@ func TestDSRouteRule(t *testing.T) {
}) })
Convey("HandleRequest()", func() { Convey("HandleRequest()", func() {
var writeErr error
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.SetCookie(w, &http.Cookie{Name: "flavor", Value: "chocolateChip"}) http.SetCookie(w, &http.Cookie{Name: "flavor", Value: "chocolateChip"})
w.WriteHeader(200) w.WriteHeader(200)
w.Write([]byte("I am the backend")) _, writeErr = w.Write([]byte("I am the backend"))
})) }))
defer backend.Close() defer backend.Close()
...@@ -533,19 +534,28 @@ func TestDSRouteRule(t *testing.T) { ...@@ -533,19 +534,28 @@ func TestDSRouteRule(t *testing.T) {
} }
Convey("When response header Set-Cookie is not set should remove proxied Set-Cookie header", func() { Convey("When response header Set-Cookie is not set should remove proxied Set-Cookie header", func() {
writeErr = nil
ctx := setupCtx(nil) ctx := setupCtx(nil)
proxy := NewDataSourceProxy(ds, plugin, ctx, "/render", &setting.Cfg{}) proxy := NewDataSourceProxy(ds, plugin, ctx, "/render", &setting.Cfg{})
proxy.HandleRequest() proxy.HandleRequest()
So(writeErr, ShouldBeNil)
So(proxy.ctx.Resp.Header().Get("Set-Cookie"), ShouldBeEmpty) So(proxy.ctx.Resp.Header().Get("Set-Cookie"), ShouldBeEmpty)
}) })
Convey("When response header Set-Cookie is set should remove proxied Set-Cookie header and restore the original Set-Cookie header", func() { Convey("When response header Set-Cookie is set should remove proxied Set-Cookie header and restore the original Set-Cookie header", func() {
writeErr = nil
ctx := setupCtx(func(w http.ResponseWriter) { ctx := setupCtx(func(w http.ResponseWriter) {
w.Header().Set("Set-Cookie", "important_cookie=important_value") w.Header().Set("Set-Cookie", "important_cookie=important_value")
}) })
proxy := NewDataSourceProxy(ds, plugin, ctx, "/render", &setting.Cfg{}) proxy := NewDataSourceProxy(ds, plugin, ctx, "/render", &setting.Cfg{})
proxy.HandleRequest() proxy.HandleRequest()
So(proxy.ctx.Resp.Header().Get("Set-Cookie"), ShouldEqual, "important_cookie=important_value")
So(writeErr, ShouldBeNil)
So(proxy.ctx.Resp.Header().Get("Set-Cookie"), ShouldEqual,
"important_cookie=important_value")
}) })
}) })
}) })
......
...@@ -41,10 +41,12 @@ func SignUp(c *m.ReqContext, form dtos.SignUpForm) Response { ...@@ -41,10 +41,12 @@ func SignUp(c *m.ReqContext, form dtos.SignUpForm) Response {
return Error(500, "Failed to create signup", err) return Error(500, "Failed to create signup", err)
} }
bus.Publish(&events.SignUpStarted{ if err := bus.Publish(&events.SignUpStarted{
Email: form.Email, Email: form.Email,
Code: cmd.Code, Code: cmd.Code,
}) }); err != nil {
return Error(500, "Failed to publish event", err)
}
metrics.MApiUserSignUpStarted.Inc() metrics.MApiUserSignUpStarted.Inc()
...@@ -85,10 +87,12 @@ func (hs *HTTPServer) SignUpStep2(c *m.ReqContext, form dtos.SignUpStep2Form) Re ...@@ -85,10 +87,12 @@ func (hs *HTTPServer) SignUpStep2(c *m.ReqContext, form dtos.SignUpStep2Form) Re
// publish signup event // publish signup event
user := &createUserCmd.Result user := &createUserCmd.Result
bus.Publish(&events.SignUpCompleted{ if err := bus.Publish(&events.SignUpCompleted{
Email: user.Email, Email: user.Email,
Name: user.NameOrFallback(), Name: user.NameOrFallback(),
}) }); err != nil {
return Error(500, "Failed to publish event", err)
}
// mark temp user as completed // mark temp user as completed
if ok, rsp := updateTempUserStatus(form.Code, m.TmpUserCompleted); !ok { if ok, rsp := updateTempUserStatus(form.Code, m.TmpUserCompleted); !ok {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment