maxmustermann/minio
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Remove panic() and handle it appropriately (#5807)

Browse source 
This is an effort to remove panic from the source. 
Add a new call called CriticialIf, that calls LogIf and exits. 
Replace panics with one of CriticalIf, FatalIf and a return of error.
This commit is contained in:
ebozduman 2018-04-19 17:24:43 -07:00 committed by kannappanr
parent 846f3e8f59
commit f16bfda2f2
21 changed files with 129 additions and 128 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
cmd/bucket-notification-handlers.go
View file

@ -220,8 +220,12 @@ func (api objectAPIHandlers) ListenBucketNotificationHandler(w http.ResponseWrit
return
}
host := xnet.MustParseHost(r.RemoteAddr)
target := target.NewHTTPClientTarget(*host, w)
host, e := xnet.ParseHost(r.RemoteAddr)
logger.CriticalIf(ctx, e)
target, e := target.NewHTTPClientTarget(*host, w)
logger.CriticalIf(ctx, e)
rulesMap := event.NewRulesMap(eventNames, pattern, target.ID())
if err := globalNotificationSys.AddRemoteTarget(bucketName, target, rulesMap); err != nil {
@ -233,7 +237,9 @@ func (api objectAPIHandlers) ListenBucketNotificationHandler(w http.ResponseWrit
defer globalNotificationSys.RemoveRemoteTarget(bucketName, target.ID())
defer globalNotificationSys.RemoveRulesMap(bucketName, rulesMap)
thisAddr := xnet.MustParseHost(GetLocalPeer(globalEndpoints))
thisAddr, e := xnet.ParseHost(GetLocalPeer(globalEndpoints))
logger.CriticalIf(ctx, e)
if err := SaveListener(objAPI, bucketName, eventNames, pattern, target.ID(), *thisAddr); err != nil {
logger.GetReqInfo(ctx).AppendTags("target", target.ID().Name)
logger.LogIf(ctx, err)

7
cmd/config-current.go
View file

@ -22,6 +22,8 @@ import (
"reflect"
"sync"
"github.com/minio/minio/cmd/logger"
"github.com/minio/minio/pkg/auth"
"github.com/minio/minio/pkg/event"
"github.com/minio/minio/pkg/event/target"
@ -167,9 +169,12 @@ func (s *serverConfig) ConfigDiff(t *serverConfig) string {
}
func newServerConfig() *serverConfig {
cred, err := auth.GetNewCredentials()
logger.FatalIf(err, "")
srvCfg := &serverConfig{
Version: serverConfigVersion,
Credential: auth.MustGetNewCredentials(),
Credential: cred,
Region: globalMinioDefaultRegion,
Browser: true,
StorageClass: storageClassConfig{

12
cmd/encryption-v1.go
View file

@ -18,6 +18,7 @@ package cmd
import (
"bytes"
"context"
"crypto/hmac"
"crypto/md5"
"crypto/rand"
@ -27,7 +28,9 @@ import (
"errors"
"io"
"net/http"
"strconv"
"github.com/minio/minio/cmd/logger"
"github.com/minio/minio/pkg/ioutil"
sha256 "github.com/minio/sha256-simd"
"github.com/minio/sio"
@ -739,10 +742,9 @@ func (li *ListPartsInfo) IsEncrypted() bool {
// DecryptedSize returns the size of the object after decryption in bytes.
// It returns an error if the object is not encrypted or marked as encrypted
// but has an invalid size.
// DecryptedSize panics if the referred object is not encrypted.
func (o *ObjectInfo) DecryptedSize() (int64, error) {
if !o.IsEncrypted() {
panic("cannot compute decrypted size of an object which is not encrypted")
return 0, errors.New("Cannot compute decrypted size of an unencrypted object")
}
size, err := sio.DecryptedSize(uint64(o.Size))
if err != nil {
@ -757,7 +759,11 @@ func (o *ObjectInfo) DecryptedSize() (int64, error) {
func (o *ObjectInfo) EncryptedSize() int64 {
size, err := sio.EncryptedSize(uint64(o.Size))
if err != nil {
panic(err) // Since AWS S3 allows parts to be 5GB at most this cannot happen - sio max. size is 256 TB
// This cannot happen since AWS S3 allows parts to be 5GB at most
// sio max. size is 256 TB
reqInfo := (&logger.ReqInfo{}).AppendTags("size", strconv.FormatUint(size, 10))
ctx := logger.SetReqInfo(context.Background(), reqInfo)
logger.CriticalIf(ctx, err)
}
return int64(size)
}

16
cmd/gateway/azure/gateway-azure.go
View file

@ -344,21 +344,21 @@ func azureToObjectError(err error, params ...string) error {
return err
}
// mustGetAzureUploadID - returns new upload ID which is hex encoded 8 bytes random value.
// getAzureUploadID - returns new upload ID which is hex encoded 8 bytes random value.
// this 8 byte restriction is needed because Azure block id has a restriction of length
// upto 8 bytes.
func mustGetAzureUploadID() string {
func getAzureUploadID() (string, error) {
var id [8]byte
n, err := io.ReadFull(rand.Reader, id[:])
if err != nil {
panic(fmt.Errorf("unable to generate upload ID for azure. %s", err))
return "", err
}
if n != len(id) {
panic(fmt.Errorf("insufficient random data (expected: %d, read: %d)", len(id), n))
return "", fmt.Errorf("Unexpected random data size. Expected: %d, read: %d)", len(id), n)
}
return hex.EncodeToString(id[:])
return hex.EncodeToString(id[:]), nil
}
// checkAzureUploadID - returns error in case of given string is upload ID.
@ -724,7 +724,11 @@ func (a *azureObjects) checkUploadIDExists(ctx context.Context, bucketName, obje
// NewMultipartUpload - Use Azure equivalent CreateBlockBlob.
func (a *azureObjects) NewMultipartUpload(ctx context.Context, bucket, object string, metadata map[string]string) (uploadID string, err error) {
uploadID = mustGetAzureUploadID()
uploadID, err = getAzureUploadID()
if err != nil {
logger.LogIf(ctx, err)
return "", err
}
metadataObject := getAzureMetadataObjectName(object, uploadID)
var jsonData []byte

5
cmd/generic-handlers.go
View file

@ -25,6 +25,7 @@ import (
"time"
humanize "github.com/dustin/go-humanize"
"github.com/minio/minio/cmd/logger"
"github.com/minio/minio/pkg/sys"
"github.com/rs/cors"
"golang.org/x/time/rate"
@ -602,9 +603,7 @@ func (h pathValidityHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// cancelled immediately.
func setRateLimitHandler(h http.Handler) http.Handler {
_, maxLimit, err := sys.GetMaxOpenFileLimit()
if err != nil {
panic(err)
}
logger.CriticalIf(context.Background(), err)
// Burst value is set to 1 to allow only maxOpenFileLimit
// requests to happen at once.
l := rate.NewLimiter(rate.Limit(maxLimit), 1)

5
cmd/jwt_test.go
View file

@ -30,7 +30,10 @@ func testAuthenticate(authType string, t *testing.T) {
t.Fatalf("unable initialize config file, %s", err)
}
defer os.RemoveAll(testPath)
cred := auth.MustGetNewCredentials()
cred, err := auth.GetNewCredentials()
if err != nil {
t.Fatalf("Error getting new credentials: %s", err)
}
globalServerConfig.SetCredential(cred)
// Define test cases.

12
cmd/logger/logger.go
View file

@ -314,7 +314,19 @@ func LogIf(ctx context.Context, err error) {
fmt.Println(output)
}
// CriticalIf :
// Like LogIf with exit
// It'll be called for fatal error conditions during run-time
func CriticalIf(ctx context.Context, err error) {
if err != nil {
LogIf(ctx, err)
os.Exit(1)
}
}
// FatalIf :
// Just fatal error message, no stack trace
// It'll be called for input validation failures
func FatalIf(err error, msg string, data ...interface{}) {
if err != nil {
if msg != "" {

19
cmd/notification.go
View file

@ -181,14 +181,15 @@ func (sys *NotificationSys) initListeners(ctx context.Context, objAPI ObjectLaye
}
defer objLock.Unlock()
reader, err := readConfig(ctx, objAPI, configFile)
if err != nil && !IsErrIgnored(err, errDiskNotFound, errNoSuchNotifications) {
return err
reader, e := readConfig(ctx, objAPI, configFile)
if e != nil && !IsErrIgnored(e, errDiskNotFound, errNoSuchNotifications) {
return e
}
listenerList := []ListenBucketNotificationArgs{}
if reader != nil {
if err = json.NewDecoder(reader).Decode(&listenerList); err != nil {
err := json.NewDecoder(reader).Decode(&listenerList)
if err != nil {
logger.LogIf(ctx, err)
return err
}
@ -201,8 +202,8 @@ func (sys *NotificationSys) initListeners(ctx context.Context, objAPI ObjectLaye
activeListenerList := []ListenBucketNotificationArgs{}
for _, args := range listenerList {
var found bool
if found, err = isLocalHost(args.Addr.Name); err != nil {
found, err := isLocalHost(args.Addr.Name)
if err != nil {
logger.GetReqInfo(ctx).AppendTags("host", args.Addr.Name)
logger.LogIf(ctx, err)
return err
@ -217,8 +218,8 @@ func (sys *NotificationSys) initListeners(ctx context.Context, objAPI ObjectLaye
return fmt.Errorf("unable to find PeerRPCClient by address %v in listener.json for bucket %v", args.Addr, bucketName)
}
var exist bool
if exist, err = rpcClient.RemoteTargetExist(bucketName, args.TargetID); err != nil {
exist, err := rpcClient.RemoteTargetExist(bucketName, args.TargetID)
if err != nil {
logger.GetReqInfo(ctx).AppendTags("targetID", args.TargetID.Name)
logger.LogIf(ctx, err)
return err
@ -231,7 +232,7 @@ func (sys *NotificationSys) initListeners(ctx context.Context, objAPI ObjectLaye
target := NewPeerRPCClientTarget(bucketName, args.TargetID, rpcClient)
rulesMap := event.NewRulesMap(args.EventNames, args.Pattern, target.ID())
if err = sys.AddRemoteTarget(bucketName, target, rulesMap); err != nil {
logger.GetReqInfo(ctx).AppendTags("targetID", target.id.Name)
logger.GetReqInfo(ctx).AppendTags("targetName", target.id.Name)
logger.LogIf(ctx, err)
return err
}

2
cmd/object-api-utils.go
View file

@ -168,7 +168,7 @@ func pathJoin(elem ...string) string {
func mustGetUUID() string {
uuid, err := uuid.New()
if err != nil {
panic(fmt.Sprintf("Random UUID generation failed. Error: %s", err))
logger.CriticalIf(context.Background(), err)
}
return uuid.String()

10
cmd/peer-rpc.go
View file

@ -104,7 +104,7 @@ func (receiver *PeerRPCReceiver) ListenBucketNotification(args *ListenBucketNoti
rulesMap := event.NewRulesMap(args.EventNames, args.Pattern, target.ID())
if err := globalNotificationSys.AddRemoteTarget(args.BucketName, target, rulesMap); err != nil {
reqInfo := &logger.ReqInfo{BucketName: target.bucketName}
reqInfo.AppendTags("target", target.id.Name)
reqInfo.AppendTags("targetName", target.id.Name)
ctx := logger.SetReqInfo(context.Background(), reqInfo)
logger.LogIf(ctx, err)
return err
@ -155,14 +155,13 @@ func (receiver *PeerRPCReceiver) SendEvent(args *SendEventArgs, reply *SendEvent
if errMap := globalNotificationSys.send(args.BucketName, args.Event, args.TargetID); len(errMap) != 0 {
var found bool
if err, found = errMap[args.TargetID]; !found {
// errMap must be zero or one element map because we sent to only one target ID.
panic(fmt.Errorf("error for target %v not found in error map %+v", args.TargetID, errMap))
return fmt.Errorf("error for target %v not found in error map %+v", args.TargetID, errMap)
}
}
if err != nil {
reqInfo := (&logger.ReqInfo{}).AppendTags("Event", args.Event.EventName.String())
reqInfo.AppendTags("target", args.TargetID.Name)
reqInfo.AppendTags("targetName", args.TargetID.Name)
ctx := logger.SetReqInfo(context.Background(), reqInfo)
logger.LogIf(ctx, err)
}
@ -275,7 +274,8 @@ func makeRemoteRPCClients(endpoints EndpointList) map[xnet.Host]*PeerRPCClient {
cred := globalServerConfig.GetCredential()
serviceEndpoint := path.Join(minioReservedBucketPath, s3Path)
for _, hostStr := range GetRemotePeers(endpoints) {
host := xnet.MustParseHost(hostStr)
host, err := xnet.ParseHost(hostStr)
logger.CriticalIf(context.Background(), err)
peerRPCClientMap[*host] = &PeerRPCClient{newAuthRPCClient(authConfig{
accessKey: cred.AccessKey,
secretKey: cred.SecretKey,

3
cmd/web-handlers.go
View file

@ -406,7 +406,8 @@ func (web webAPIHandlers) GenerateAuth(r *http.Request, args *WebGenericArgs, re
if !isHTTPRequestValid(r) {
return toJSONError(errAuthentication)
}
cred := auth.MustGetNewCredentials()
cred, err := auth.GetNewCredentials()
logger.CriticalIf(context.Background(), err)
reply.AccessKey = cred.AccessKey
reply.SecretKey = cred.SecretKey
reply.UIVersion = browser.UIVersion

17
cmd/xl-v1-metadata.go
View file

@ -21,6 +21,7 @@ import (
"crypto"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"hash"
"path"
@ -42,7 +43,7 @@ var DefaultBitrotAlgorithm = HighwayHash256
func init() {
hh256Key, err := hex.DecodeString(magicHighwayHash256Key)
if err != nil || len(hh256Key) != highwayhash.Size {
panic("Failed to decode fixed magic HighwayHash256 key: Please report this bug at https://github.com/minio/minio/issues")
logger.CriticalIf(context.Background(), errors.New("Failed to decode fixed magic HighwayHash256 key. Please report this bug at https://github.com/minio/minio/issues"))
}
newBLAKE2b := func() hash.Hash {
@ -80,11 +81,12 @@ var bitrotAlgorithms = map[BitrotAlgorithm]string{
HighwayHash256: "highwayhash256",
}
// New returns a new hash.Hash calculating the given bitrot algorithm. New panics
// if the algorithm is not supported or not linked into the binary.
// New returns a new hash.Hash calculating the given bitrot algorithm.
// New logs error and exits if the algorithm is not supported or not
// linked into the binary.
func (a BitrotAlgorithm) New() hash.Hash {
if _, ok := bitrotAlgorithms[a]; !ok {
panic(fmt.Sprintf("bitrot algorithm #%d is not supported", a))
logger.CriticalIf(context.Background(), errors.New("Unsupported bitrot algorithm"))
}
return crypto.Hash(a).New()
}
@ -98,10 +100,11 @@ func (a BitrotAlgorithm) Available() bool {
// String returns the string identifier for a given bitrot algorithm.
// If the algorithm is not supported String panics.
func (a BitrotAlgorithm) String() string {
if name, ok := bitrotAlgorithms[a]; ok {
return name
name, ok := bitrotAlgorithms[a]
if !ok {
logger.CriticalIf(context.Background(), errors.New("Unsupported bitrot algorithm"))
}
panic(fmt.Sprintf("bitrot algorithm #%d is not supported", a))
return name
}
// BitrotAlgorithmFromString returns a bitrot algorithm from the given string representation.

27
pkg/auth/credentials.go
View file

@ -81,30 +81,37 @@ func (cred Credentials) Equal(ccred Credentials) bool {
return cred.AccessKey == ccred.AccessKey && subtle.ConstantTimeCompare([]byte(cred.SecretKey), []byte(ccred.SecretKey)) == 1
}
// MustGetNewCredentials generates and returns new credential.
func MustGetNewCredentials() (cred Credentials) {
readBytes := func(size int) (data []byte) {
// GetNewCredentials generates and returns new credential.
func GetNewCredentials() (cred Credentials, err error) {
readBytes := func(size int) (data []byte, err error) {
data = make([]byte, size)
if n, err := rand.Read(data); err != nil {
panic(err)
var n int
if n, err = rand.Read(data); err != nil {
return nil, err
} else if n != size {
panic(fmt.Errorf("not enough data read. expected: %v, got: %v", size, n))
return nil, fmt.Errorf("Not enough data. Expected to read: %v bytes, got: %v bytes", size, n)
}
return
return data, nil
}
// Generate access key.
keyBytes := readBytes(accessKeyMaxLen)
keyBytes, err := readBytes(accessKeyMaxLen)
if err != nil {
return cred, err
}
for i := 0; i < accessKeyMaxLen; i++ {
keyBytes[i] = alphaNumericTable[keyBytes[i]%alphaNumericTableLen]
}
cred.AccessKey = string(keyBytes)
// Generate secret key.
keyBytes = readBytes(secretKeyMaxLen)
keyBytes, err = readBytes(secretKeyMaxLen)
if err != nil {
return cred, err
}
cred.SecretKey = string([]byte(base64.StdEncoding.EncodeToString(keyBytes))[:secretKeyMaxLen])
return cred
return cred, nil
}
// CreateCredentials returns new credential with the given access key and secret key.

18
pkg/auth/credentials_test.go
View file

@ -54,8 +54,11 @@ func TestIsSecretKeyValid(t *testing.T) {
}
}
func TestMustGetNewCredentials(t *testing.T) {
cred := MustGetNewCredentials()
func TestGetNewCredentials(t *testing.T) {
cred, err := GetNewCredentials()
if err != nil {
t.Fatalf("Failed to get a new credential")
}
if !cred.IsValid() {
t.Fatalf("Failed to get new valid credential")
}
@ -106,7 +109,14 @@ func TestCreateCredentials(t *testing.T) {
}
func TestCredentialsEqual(t *testing.T) {
cred := MustGetNewCredentials()
cred, err := GetNewCredentials()
if err != nil {
t.Fatalf("Failed to get a new credential")
}
cred2, err := GetNewCredentials()
if err != nil {
t.Fatalf("Failed to get a new credential")
}
testCases := []struct {
cred Credentials
ccred Credentials
@ -119,7 +129,7 @@ func TestCredentialsEqual(t *testing.T) {
// Empty credentials.
{Credentials{}, cred, false},
// Two different credentialss
{cred, MustGetNewCredentials(), false},
{cred, cred2, false},
// Access key is different in credentials to compare.
{cred, Credentials{AccessKey: "myuser", SecretKey: cred.SecretKey}, false},
// Secret key is different in credentials to compare.

17
pkg/event/target/httpclient.go
View file

@ -19,7 +19,6 @@ package target
import (
"encoding/json"
"errors"
"fmt"
"net/http"
"sync/atomic"
"time"
@ -118,24 +117,28 @@ func (target *HTTPClientTarget) Close() error {
return nil
}
func mustGetNewUUID() string {
func getNewUUID() (string, error) {
uuid, err := uuid.New()
if err != nil {
panic(fmt.Sprintf("%s. Unable to generate random UUID", err))
return "", err
}
return uuid.String()
return uuid.String(), nil
}
// NewHTTPClientTarget - creates new HTTP client target.
func NewHTTPClientTarget(host xnet.Host, w http.ResponseWriter) *HTTPClientTarget {
func NewHTTPClientTarget(host xnet.Host, w http.ResponseWriter) (*HTTPClientTarget, error) {
uuid, err := getNewUUID()
if err != nil {
return nil, err
}
c := &HTTPClientTarget{
id: event.TargetID{"httpclient" + "+" + mustGetNewUUID() + "+" + host.Name, host.Port.String()},
id: event.TargetID{"httpclient" + "+" + uuid + "+" + host.Name, host.Port.String()},
w: w,
eventCh: make(chan []byte),
DoneCh: make(chan struct{}),
stopCh: make(chan struct{}),
}
c.start()
return c
return c, nil
}

5
pkg/event/target/nats.go
View file

@ -112,7 +112,10 @@ func NewNATSTarget(id string, args NATSArgs) (*NATSTarget, error) {
clientID := args.Streaming.ClientID
if clientID == "" {
clientID = mustGetNewUUID()
clientID, err = getNewUUID()
if err != nil {
return nil, err
}
}
connOpts := []stan.Option{stan.NatsURL(addressURL)}

9
pkg/net/host.go
View file

@ -139,12 +139,3 @@ func ParseHost(s string) (*Host, error) {
IsPortSet: isPortSet,
}, nil
}
// MustParseHost - parses given string to Host, else panics.
func MustParseHost(s string) *Host {
host, err := ParseHost(s)
if err != nil {
panic(err)
}
return host
}

27
pkg/net/host_test.go
View file

@ -207,30 +207,3 @@ func TestParseHost(t *testing.T) {
}
}
}
func TestMustParseHost(t *testing.T) {
testCases := []struct {
s string
expectedHost *Host
}{
{"play", &Host{"play", 0, false}},
{"play:0", &Host{"play", 0, true}},
{"play:9000", &Host{"play", 9000, true}},
{"play.minio.io", &Host{"play.minio.io", 0, false}},
{"play.minio.io:9000", &Host{"play.minio.io", 9000, true}},
{"147.75.201.93", &Host{"147.75.201.93", 0, false}},
{"147.75.201.93:9000", &Host{"147.75.201.93", 9000, true}},
{"play12", &Host{"play12", 0, false}},
{"12play", &Host{"12play", 0, false}},
{"play-minio-io", &Host{"play-minio-io", 0, false}},
{"play--minio.io", &Host{"play--minio.io", 0, false}},
}
for i, testCase := range testCases {
host := MustParseHost(testCase.s)
if !reflect.DeepEqual(host, testCase.expectedHost) {
t.Fatalf("test %v: host: expected: %#v, got: %#v", i+1, testCase.expectedHost, host)
}
}
}

10
pkg/net/port.go
View file

@ -42,13 +42,3 @@ func ParsePort(s string) (p Port, err error) {
return Port(i), nil
}
// MustParsePort - parses string into Port, else panics
func MustParsePort(s string) Port {
p, err := ParsePort(s)
if err != nil {
panic(err)
}
return p
}

19
pkg/net/port_test.go
View file

@ -71,22 +71,3 @@ func TestParsePort(t *testing.T) {
}
}
}
func TestMustParsePort(t *testing.T) {
testCases := []struct {
s string
expectedPort Port
}{
{"0", Port(0)},
{"9000", Port(9000)},
{"65535", Port(65535)},
}
for i, testCase := range testCases {
port := MustParsePort(testCase.s)
if port != testCase.expectedPort {
t.Fatalf("test %v: error: port: %v, got: %v", i+1, testCase.expectedPort, port)
}
}
}

5
pkg/net/url.go
View file

@ -35,7 +35,10 @@ func (u URL) IsEmpty() bool {
func (u URL) String() string {
// if port number 80 and 443, remove for http and https scheme respectively
if u.Host != "" {
host := MustParseHost(u.Host)
host, err := ParseHost(u.Host)
if err != nil {
panic(err)
}
switch {
case u.Scheme == "http" && host.Port == 80:
fallthrough