Fix other Go Report Card warnings (#423)

Fix other warnings that we might run into on Go Report Card. * go vet: fix warnings about unkeyed field initialization Add field names to struct initializers, mostly these are our private key structs. * go ineffassign: fix ineffective assignments This is a neat tool that mostly catches unhandled errors (assignments to err variables that are never read). Add appropriate error handling to deal with these warnings. Pay attention to the subtle difference between = and := assignments. * go cyclo: reduce cyclomatic complexity Some functions already had it too high, some got it too high after additional error handling. We can't do anything about it other than split the big functions into smaller ones and introduce helpers to avoid too many conditionals in a single function. * go misspell: fix a couple of typos * go fmt: reformat the code * Improve error handling in Themis server examples - Split the code in Secure Message example in the same way we do for Secure Session. - Improve the quit message a but. Without quotes it's not intuitive that you have to literally type in q-u-i-t to quit cleanly. - Unify error reporting in the code and output messages to stderr. - Exit with non-zero status code to indicate failure. - Make sure that errors are printed out nicely.
cossacklabs · Mar 12, 2019 · c072a00 · c072a00
1 parent 9ab951e
commit c072a00
Show file tree

Hide file tree

Showing 11 changed files with 186 additions and 81 deletions.
diff --git a/docs/examples/Themis-server/go/smessage_server.go b/docs/examples/Themis-server/go/smessage_server.go
@@ -38,52 +38,93 @@ func sendMessage(message []byte, endpoint string) ([]byte, error) {
 	return data, nil
 }
 
-func main() {
-	inputBuffer := bufio.NewReader(os.Stdin)
+func createSecureMessage(inputBuffer *bufio.Reader) (*message.SecureMessage, string, error) {
 	fmt.Println("Type your settings from https://themis.cossacklabs.com/interactive-simulator/setup/")
 
 	fmt.Println("JSON endpoint: ")
 	endpoint, err := inputBuffer.ReadString('\n')
+	if err != nil {
+		err = fmt.Errorf("Failed to read JSON endpoint: %s", err)
+		return nil, "", err
+	}
 	endpoint = strings.TrimRight(endpoint, "\n\r")
 
 	fmt.Println("Your private key in base64 format:")
 	clientPrivate, err := inputBuffer.ReadBytes('\n')
+	if err != nil {
+		err = fmt.Errorf("Failed to read user private key: %s", err)
+		return nil, "", err
+	}
 	clientPrivate, err = base64.StdEncoding.DecodeString(string(clientPrivate))
 	if err != nil {
-		fmt.Println("Incorrect base64 format for private key")
-		return
+		err = fmt.Errorf("Incorrect base64 format for private key: %s", err)
+		return nil, "", err
 	}
 
 	fmt.Println("Server public key in base64 format:")
 	serverPublic, err := inputBuffer.ReadBytes('\n')
+	if err != nil {
+		err = fmt.Errorf("Failed to read server public key: %s", err)
+		return nil, "", err
+	}
+
 	serverPublic = bytes.TrimRight(serverPublic, "\r\n")
 	serverPublic, err = base64.StdEncoding.DecodeString(string(serverPublic))
+	if err != nil {
+		err = fmt.Errorf("Incorrect base64 format for public key: %s", err)
+		return nil, "", err
+	}
+
 	secureMessage := message.New(
-		&keys.PrivateKey{bytes.TrimRight(clientPrivate, "\r\n")},
-		&keys.PublicKey{serverPublic})
+		&keys.PrivateKey{Value: bytes.TrimRight(clientPrivate, "\r\n")},
+		&keys.PublicKey{Value: serverPublic})
+
+	return secureMessage, endpoint, nil
+}
 
+func runSecureMessage(secureMessage *message.SecureMessage, endpoint string, inputBuffer *bufio.Reader) error {
 	for {
-		fmt.Println("Print message to send (or quit to stop):")
+		fmt.Println("Print message to send (or \"quit\" to stop):")
 		line, _, err := inputBuffer.ReadLine()
 		if err != nil {
-			fmt.Println(err)
-			return
+			err = fmt.Errorf("Failed to read message: %s", err)
+			return err
 		}
 		if bytes.Equal(line, []byte("quit")) {
-			return
+			return nil
 		}
 
 		wrapped, err := secureMessage.Wrap(line)
 		if err != nil {
-			fmt.Println("Error in wraping", err)
-			return
+			err = fmt.Errorf("Failed to encrypt message: %s", err)
+			return err
 		}
 		data, err := sendMessage(wrapped, endpoint)
 		if err != nil {
-			fmt.Println("Error occurred:", err)
-			return
+			err = fmt.Errorf("Failed to send message: %s", err)
+			return err
 		}
 		unwrapped, err := secureMessage.Unwrap(data)
+		if err != nil {
+			err = fmt.Errorf("Failed to decrypt message: %s", err)
+			return err
+		}
 		fmt.Println(string(unwrapped))
 	}
 }
+
+func main() {
+	inputBuffer := bufio.NewReader(os.Stdin)
+
+	secureMessage, endpoint, err := createSecureMessage(inputBuffer)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+
+	err = runSecureMessage(secureMessage, endpoint, inputBuffer)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+}
diff --git a/docs/examples/Themis-server/go/ssession_server.go b/docs/examples/Themis-server/go/ssession_server.go
@@ -22,7 +22,7 @@ type clientTransportCallback struct {
 
 func (clb *clientTransportCallback) GetPublicKeyForId(ss *session.SecureSession, id []byte) *keys.PublicKey {
 	if bytes.Equal(id, clb.serverID) {
-		return &keys.PublicKey{clb.serverPublic}
+		return &keys.PublicKey{Value: clb.serverPublic}
 	}
 	return nil
 
@@ -81,31 +81,55 @@ func clientService(client *session.SecureSession, ch chan []byte, finCh chan int
 	finCh <- 1
 }
 
-func main() {
-	inputBuffer := bufio.NewReader(os.Stdin)
+func createSecureSession(inputBuffer *bufio.Reader) (*session.SecureSession, string, error) {
 	fmt.Println("Type your settings from https://themis.cossacklabs.com/interactive-simulator/setup/")
 
 	fmt.Println("JSON endpoint: ")
 	endpoint, err := inputBuffer.ReadString('\n')
+	if err != nil {
+		err = fmt.Errorf("Failed to read endpoint URL: %s", err)
+		return nil, "", err
+	}
 	endpoint = strings.TrimRight(endpoint, "\n\r")
 
 	fmt.Println("Your private key in base64 format:")
 	clientPrivate, err := inputBuffer.ReadBytes('\n')
+	if err != nil {
+		err = fmt.Errorf("Failed to read user private key: %s", err)
+		return nil, "", err
+	}
 	clientPrivate, err = base64.StdEncoding.DecodeString(string(clientPrivate))
 	if err != nil {
-		fmt.Println("Incorrect base64 format for private key")
-		return
+		err = fmt.Errorf("Incorrect base64 format for private key: %s", err)
+		return nil, "", err
 	}
 
-	fmt.Println("User_id:")
+	fmt.Println("User ID:")
 	clientID, err := inputBuffer.ReadBytes('\n')
+	if err != nil {
+		err = fmt.Errorf("Failed to read user ID: %s", err)
+		return nil, "", err
+	}
 
-	fmt.Println("Server_id:")
+	fmt.Println("Server ID:")
 	serverID, err := inputBuffer.ReadBytes('\n')
+	if err != nil {
+		err = fmt.Errorf("Failed to read server ID: %s", err)
+		return nil, "", err
+	}
 
 	fmt.Println("Server public key in base64 format:")
 	serverPublic, err := inputBuffer.ReadBytes('\n')
+	if err != nil {
+		err = fmt.Errorf("Failed to read server public key: %s", err)
+		return nil, "", err
+	}
 	serverPublic, err = base64.StdEncoding.DecodeString(string(serverPublic))
+	if err != nil {
+		err = fmt.Errorf("Incorrect base64 format for public key: %s", err)
+		return nil, "", err
+	}
+
 	// init callback structure
 	cb := clientTransportCallback{
 		serverPublic,
@@ -114,49 +138,79 @@ func main() {
 	// create session object
 	clientSession, err := session.New(
 		bytes.TrimRight(clientID, "\r\n"),
-		&keys.PrivateKey{bytes.TrimRight(clientPrivate, "\r\n")},
+		&keys.PrivateKey{Value: bytes.TrimRight(clientPrivate, "\r\n")},
 		&cb)
 	if err != nil {
-		fmt.Println("Session creation error")
-		return
+		err = fmt.Errorf("Cannot create Secure Session: %s", err)
+		return nil, "", err
 	}
 
+	return clientSession, endpoint, nil
+}
+
+func runSecureSession(clientSession *session.SecureSession, endpoint string, inputBuffer *bufio.Reader) error {
 	ch := make(chan []byte)
 	quitChannel := make(chan int)
 	go clientService(clientSession, ch, quitChannel)
 	isEstablished := false
+
 	fmt.Println("Initialize session")
 	for !isEstablished {
 		select {
 		case data := <-ch:
 			data, err := sendMessage(data, endpoint)
 			if err != nil {
-				fmt.Println("Error -", err)
-				return
+				err = fmt.Errorf("Failed to send message: %s", err)
+				return err
 			}
 			ch <- data
 		case <-quitChannel:
 			isEstablished = true
 		}
 	}
 	fmt.Println("Session established")
+
 	for {
-		fmt.Println("Print message to send (or quit to stop):")
+		fmt.Println("Print message to send (or \"quit\" to stop):")
 		line, _, err := inputBuffer.ReadLine()
 		if err != nil {
-			fmt.Println(err)
-			return
+			err = fmt.Errorf("Failed to read message: %s", err)
+			return err
 		}
 		if bytes.Equal(line, []byte("quit")) {
-			return
+			return nil
 		}
 		wrapped, err := clientSession.Wrap(line)
+		if err != nil {
+			err = fmt.Errorf("Failed to read message: %s", err)
+			return err
+		}
 		data, err := sendMessage(wrapped, endpoint)
 		if err != nil {
-			fmt.Println("Error occurred:", err)
-			return
+			err = fmt.Errorf("Failed to send message: %s", err)
+			return err
 		}
 		unwrapped, _, err := clientSession.Unwrap(data)
+		if err != nil {
+			err = fmt.Errorf("Failed to decrypt message: %s", err)
+			return err
+		}
 		fmt.Println(string(unwrapped))
 	}
 }
+
+func main() {
+	inputBuffer := bufio.NewReader(os.Stdin)
+
+	clientSession, endpoint, err := createSecureSession(inputBuffer)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+
+	err = runSecureSession(clientSession, endpoint, inputBuffer)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+}
diff --git a/docs/examples/go/secure_message.go b/docs/examples/go/secure_message.go
@@ -18,13 +18,13 @@ func main() {
 		fmt.Println("error decoding private key")
 		return
 	}
-	pr := keys.PrivateKey{decodedKey}
+	pr := keys.PrivateKey{Value: decodedKey}
 	decodedKey, err = base64.StdEncoding.DecodeString(os.Args[3])
 	if nil != err {
 		fmt.Println("error decoding private key")
 		return
 	}
-	pu := keys.PublicKey{decodedKey}
+	pu := keys.PublicKey{Value: decodedKey}
 	sm := message.New(&pr, &pu)
 	if "enc" == os.Args[1] {
 		wrapped, err := sm.Wrap([]byte(os.Args[4]))

diff --git a/docs/examples/go/secure_session_client.go b/docs/examples/go/secure_session_client.go
@@ -16,7 +16,7 @@ func (clb *callbacks) GetPublicKeyForId(ss *session.SecureSession, id []byte) *k
 	if nil != err {
 		return nil
 	}
-	return &keys.PublicKey{decodedID}
+	return &keys.PublicKey{Value: decodedID}
 }
 
 func (clb *callbacks) StateChanged(ss *session.SecureSession, state int) {

diff --git a/docs/examples/go/secure_session_server.go b/docs/examples/go/secure_session_server.go
@@ -16,7 +16,7 @@ func (clb *callbacks) GetPublicKeyForId(ss *session.SecureSession, id []byte) *k
 	if nil != err {
 		return nil
 	}
-	return &keys.PublicKey{decodedID}
+	return &keys.PublicKey{Value: decodedID}
 }
 
 func (clb *callbacks) StateChanged(ss *session.SecureSession, state int) {

diff --git a/gothemis/cell/cell.go b/gothemis/cell/cell.go
@@ -149,30 +149,34 @@ func New(key []byte, mode int) *SecureCell {
 	return &SecureCell{key, mode}
 }
 
+func missing(data []byte) bool {
+	return data == nil || len(data) == 0
+}
+
 // Protect encrypts or signs data with optional user context (depending on the Cell mode).
 func (sc *SecureCell) Protect(data []byte, context []byte) ([]byte, []byte, error) {
 	if (sc.mode < CELL_MODE_SEAL) || (sc.mode > CELL_MODE_CONTEXT_IMPRINT) {
 		return nil, nil, errors.New("Invalid mode specified")
 	}
 
-	if nil == sc.key || 0 == len(sc.key) {
+	if missing(sc.key) {
 		return nil, nil, errors.New("Master key was not provided")
 	}
 
-	if nil == data || 0 == len(data) {
+	if missing(data) {
 		return nil, nil, errors.New("Data was not provided")
 	}
 
 	if CELL_MODE_CONTEXT_IMPRINT == sc.mode {
-		if nil == context || 0 == len(context) {
+		if missing(context) {
 			return nil, nil, errors.New("Context is mandatory for context imprint mode")
 		}
 	}
 
 	var ctx unsafe.Pointer
 	var ctxLen C.size_t
 
-	if nil != context && 0 < len(context) {
+	if !missing(context) {
 		ctx = unsafe.Pointer(&context[0])
 		ctxLen = C.size_t(len(context))
 	}
@@ -223,35 +227,35 @@ func (sc *SecureCell) Unprotect(protectedData []byte, additionalData []byte, con
 		return nil, errors.New("Invalid mode specified")
 	}
 
-	if nil == sc.key || 0 == len(sc.key) {
+	if missing(sc.key) {
 		return nil, errors.New("Master key was not provided")
 	}
 
-	if nil == protectedData || 0 == len(protectedData) {
+	if missing(protectedData) {
 		return nil, errors.New("Data was not provided")
 	}
 
 	if CELL_MODE_CONTEXT_IMPRINT == sc.mode {
-		if nil == context || 0 == len(context) {
+		if missing(context) {
 			return nil, errors.New("Context is mandatory for context imprint mode")
 		}
 	}
 
 	if CELL_MODE_TOKEN_PROTECT == sc.mode {
-		if nil == additionalData || 0 == len(additionalData) {
+		if missing(additionalData) {
 			return nil, errors.New("Additional data is mandatory for token protect mode")
 		}
 	}
 
 	var add, ctx unsafe.Pointer = nil, nil
 	var addLen, ctxLen C.size_t = 0, 0
 
-	if nil != additionalData && 0 < len(additionalData) {
+	if !missing(additionalData) {
 		add = unsafe.Pointer(&additionalData[0])
 		addLen = C.size_t(len(additionalData))
 	}
 
-	if nil != context && 0 < len(context) {
+	if !missing(context) {
 		ctx = unsafe.Pointer(&context[0])
 		ctxLen = C.size_t(len(context))
 	}