check for nil state in s3 client Get

The S3 client can return (nil, nil) when the remote state doesn't exist.
The caused a nil pointer dereference when checking the payload.MD5
against the expected value.

This can happen if the remote state was manually removed, but the digest
entry was left in the DynamoDB table.

backend/remote-state/s3/client.go

@@ -58,11 +58,19 @@ func (c *RemoteClient) Get() (payload *remote.Payload, err error) {
 			return nil, err
 		}
 
+		// If the remote state was manually removed the payload will be nil,
+		// but if there's still a digest entry for that state we will still try
+		// to compare the MD5 below.
+		var digest []byte
+		if payload != nil {
+			digest = payload.MD5
+		}
+
 		// verify that this state is what we expect
 		if expected, err := c.getMD5(); err != nil {
 			log.Printf("[WARNING] failed to fetch state md5: %s", err)
-		} else if len(expected) > 0 && !bytes.Equal(expected, payload.MD5) {
+		} else if len(expected) > 0 && !bytes.Equal(expected, digest) {
-			log.Printf("[WARNING] state md5 mismatch: expected '%x', got '%x'", expected, payload.MD5)
+			log.Printf("[WARNING] state md5 mismatch: expected '%x', got '%x'", expected, digest)
 
 			if testChecksumHook != nil {
 				testChecksumHook()
@@ -74,7 +82,7 @@ func (c *RemoteClient) Get() (payload *remote.Payload, err error) {
 				continue
 			}
 
-			return nil, fmt.Errorf(errBadChecksumFmt, payload.MD5)
+			return nil, fmt.Errorf(errBadChecksumFmt, digest)
 		}
 
 		break

backend/remote-state/s3/client_test.go

@@ -167,6 +167,8 @@ func TestRemoteClient_clientMD5(t *testing.T) {
 		"lock_table": bucketName,
 	}).(*Backend)
 
+	createS3Bucket(t, b.s3Client, bucketName)
+	defer deleteS3Bucket(t, b.s3Client, bucketName)
 	createDynamoDBTable(t, b.dynClient, bucketName)
 	defer deleteDynamoDBTable(t, b.dynClient, bucketName)
 
@@ -263,8 +265,8 @@ func TestRemoteClient_stateChecksum(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// put the old state in place of the new, without updating the checksum
+	// put an empty state in place to check for panics during get
-	if err := client2.Put(oldState.Bytes()); err != nil {
+	if err := client2.Put([]byte{}); err != nil {
 		t.Fatal(err)
 	}
 
@@ -278,7 +280,18 @@ func TestRemoteClient_stateChecksum(t *testing.T) {
 	consistencyRetryTimeout = 0
 	consistencyRetryPollInterval = 0
 
-	// fetching the state through client1 should now error out due to a
+	// fetching an empty state through client1 should now error out due to a
+	// mismatched checksum.
+	if _, err := client1.Get(); !strings.HasPrefix(err.Error(), errBadChecksumFmt[:80]) {
+		t.Fatalf("expected state checksum error: got %s", err)
+	}
+
+	// put the old state in place of the new, without updating the checksum
+	if err := client2.Put(oldState.Bytes()); err != nil {
+		t.Fatal(err)
+	}
+
+	// fetching the wrong state through client1 should now error out due to a
 	// mismatched checksum.
 	if _, err := client1.Get(); !strings.HasPrefix(err.Error(), errBadChecksumFmt[:80]) {
 		t.Fatalf("expected state checksum error: got %s", err)