forked from TrueCloudLab/distribution
Merge pull request #548 from duglin/MoveErrors
Move ErrorCode logic to new errcode package
This commit is contained in:
commit
e57e731821
20 changed files with 768 additions and 665 deletions
|
@ -14,6 +14,7 @@ import (
|
|||
"github.com/docker/distribution/configuration"
|
||||
ctxu "github.com/docker/distribution/context"
|
||||
"github.com/docker/distribution/notifications"
|
||||
"github.com/docker/distribution/registry/api/errcode"
|
||||
"github.com/docker/distribution/registry/api/v2"
|
||||
"github.com/docker/distribution/registry/auth"
|
||||
registrymiddleware "github.com/docker/distribution/registry/middleware/registry"
|
||||
|
@ -372,12 +373,11 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler {
|
|||
|
||||
switch err := err.(type) {
|
||||
case distribution.ErrRepositoryUnknown:
|
||||
context.Errors.Push(v2.ErrorCodeNameUnknown, err)
|
||||
context.Errors = append(context.Errors, v2.ErrorCodeNameUnknown.WithDetail(err))
|
||||
case distribution.ErrRepositoryNameInvalid:
|
||||
context.Errors.Push(v2.ErrorCodeNameInvalid, err)
|
||||
context.Errors = append(context.Errors, v2.ErrorCodeNameInvalid.WithDetail(err))
|
||||
}
|
||||
|
||||
w.WriteHeader(http.StatusBadRequest)
|
||||
serveJSON(w, context.Errors)
|
||||
return
|
||||
}
|
||||
|
@ -390,8 +390,8 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler {
|
|||
context.Repository, err = applyRepoMiddleware(context.Repository, app.Config.Middleware["repository"])
|
||||
if err != nil {
|
||||
ctxu.GetLogger(context).Errorf("error initializing repository middleware: %v", err)
|
||||
context.Errors.Push(v2.ErrorCodeUnknown, err)
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
context.Errors = append(context.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
|
||||
|
||||
serveJSON(w, context.Errors)
|
||||
return
|
||||
}
|
||||
|
@ -402,23 +402,33 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler {
|
|||
// own errors if they need different behavior (such as range errors
|
||||
// for layer upload).
|
||||
if context.Errors.Len() > 0 {
|
||||
if context.Value("http.response.status") == 0 {
|
||||
// TODO(stevvooe): Getting this value from the context is a
|
||||
// bit of a hack. We can further address with some of our
|
||||
// future refactoring.
|
||||
w.WriteHeader(http.StatusBadRequest)
|
||||
}
|
||||
app.logError(context, context.Errors)
|
||||
|
||||
serveJSON(w, context.Errors)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func (app *App) logError(context context.Context, errors v2.Errors) {
|
||||
for _, e := range errors.Errors {
|
||||
c := ctxu.WithValue(context, "err.code", e.Code)
|
||||
c = ctxu.WithValue(c, "err.message", e.Message)
|
||||
c = ctxu.WithValue(c, "err.detail", e.Detail)
|
||||
func (app *App) logError(context context.Context, errors errcode.Errors) {
|
||||
for _, e1 := range errors {
|
||||
var c ctxu.Context
|
||||
|
||||
switch e1.(type) {
|
||||
case errcode.Error:
|
||||
e, _ := e1.(errcode.Error)
|
||||
c = ctxu.WithValue(context, "err.code", e.Code)
|
||||
c = ctxu.WithValue(c, "err.message", e.Code.Message())
|
||||
c = ctxu.WithValue(c, "err.detail", e.Detail)
|
||||
case errcode.ErrorCode:
|
||||
e, _ := e1.(errcode.ErrorCode)
|
||||
c = ctxu.WithValue(context, "err.code", e)
|
||||
c = ctxu.WithValue(c, "err.message", e.Message())
|
||||
default:
|
||||
// just normal go 'error'
|
||||
c = ctxu.WithValue(context, "err.code", errcode.ErrorCodeUnknown)
|
||||
c = ctxu.WithValue(c, "err.message", e1.Error())
|
||||
}
|
||||
|
||||
c = ctxu.WithLogger(c, ctxu.GetLogger(c,
|
||||
"err.code",
|
||||
"err.message",
|
||||
|
@ -471,11 +481,10 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
|
|||
// base route is accessed. This section prevents us from making
|
||||
// that mistake elsewhere in the code, allowing any operation to
|
||||
// proceed.
|
||||
w.Header().Set("Content-Type", "application/json; charset=utf-8")
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
|
||||
var errs v2.Errors
|
||||
errs.Push(v2.ErrorCodeUnauthorized)
|
||||
var errs errcode.Errors
|
||||
errs = append(errs, v2.ErrorCodeUnauthorized)
|
||||
|
||||
serveJSON(w, errs)
|
||||
return fmt.Errorf("forbidden: no repository name")
|
||||
}
|
||||
|
@ -485,11 +494,20 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
|
|||
if err != nil {
|
||||
switch err := err.(type) {
|
||||
case auth.Challenge:
|
||||
// NOTE(duglin):
|
||||
// Since err.ServeHTTP will set the HTTP status code for us
|
||||
// we need to set the content-type here. The serveJSON
|
||||
// func will try to do it but it'll be too late at that point.
|
||||
// I would have have preferred to just have the auth.Challenge
|
||||
// ServerHTTP func just add the WWW-Authenticate header and let
|
||||
// serveJSON set the HTTP status code and content-type but I wasn't
|
||||
// sure if that's an ok design change. STEVVOOE ?
|
||||
w.Header().Set("Content-Type", "application/json; charset=utf-8")
|
||||
|
||||
err.ServeHTTP(w, r)
|
||||
|
||||
var errs v2.Errors
|
||||
errs.Push(v2.ErrorCodeUnauthorized, accessRecords)
|
||||
var errs errcode.Errors
|
||||
errs = append(errs, v2.ErrorCodeUnauthorized.WithDetail(accessRecords))
|
||||
serveJSON(w, errs)
|
||||
default:
|
||||
// This condition is a potential security problem either in
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue