Fix base64 decoding for debug log statements (#12)

* Expand path decoding slightly

Add function to decode an entire path and change result when a decode
error occurs to better fix use-case.

Also add tests for decoding a path and error case.

* Fixup logic in FinishedHashingFile handler

Use new decoding function so we hopefully stop getting failed to decode
errors. The original error was cropping up because it was using standard
base64 decoding while the encoder uses URL (and file system) safe base64
encoding. Those encoder types have different alphabets that they
translate to, thus the errors.

src/internal/kopia/path_encoder.go

@@ -3,6 +3,7 @@ package kopia
 import (
 	"encoding/base64"
 	"path"
+	"strings"
 
 	"github.com/alcionai/clues"
 )
@@ -22,19 +23,42 @@ func encodeElements(elements ...string) []string {
 	return encoded
 }
 
+// decodePath splits inputPath on the path separator and returns the base64
+// decoding of each element in the path. If an error occurs then returns a mixed
+// set of encoded and decoded elements and an error with information about each
+// element that failed decoding.
+func decodePath(inputPath string) ([]string, error) {
+	res, err := decodeElements(strings.Split(inputPath, "/")...)
+	return res, clues.Stack(err).OrNil()
+}
+
+// decodeElements returns the base64 decoding of each input element. If any
+// element fails to decode it returns a mix of encoded (failed) decoded elements
+// and an error.
 func decodeElements(elements ...string) ([]string, error) {
-	decoded := make([]string, 0, len(elements))
+	var (
+		errs    *clues.Err
+		decoded = make([]string, 0, len(elements))
+	)
 
 	for _, e := range elements {
-		bs, err := encoder.DecodeString(e)
+		decodedBytes, err := encoder.DecodeString(e)
+		// Make an additional string variable so we can just assign to it if there
+		// was an error. This avoids a continue in the error check below.
+		decodedElement := string(decodedBytes)
+
 		if err != nil {
-			return nil, clues.Wrap(err, "decoding element").With("element", e)
+			errs = clues.Stack(
+				errs,
+				clues.Wrap(err, "decoding element").With("element", e))
+			// Set bs to the input value so it gets returned in its encoded form.
+			decodedElement = e
 		}
 
-		decoded = append(decoded, string(bs))
+		decoded = append(decoded, decodedElement)
 	}
 
-	return decoded, nil
+	return decoded, errs.OrNil()
 }
 
 // encodeAsPath takes a set of elements and returns the concatenated elements as

src/internal/kopia/path_encoder_test.go

@@ -36,7 +36,82 @@ func (suite *PathEncoderSuite) TestEncodeDecode() {
 	assert.Equal(t, elements, decoded)
 }
 
-func (suite *PathEncoderSuite) TestEncodeAsPathDecode() {
+func (suite *PathEncoderSuite) TestEncodeAsPathDecodePath() {
+	table := []struct {
+		name     string
+		elements []string
+		expected []string
+	}{
+		{
+			name:     "MultipleElements",
+			elements: []string{"these", "are", "some", "path", "elements"},
+			expected: []string{"these", "are", "some", "path", "elements"},
+		},
+		{
+			name:     "SingleElement",
+			elements: []string{"elements"},
+			expected: []string{"elements"},
+		},
+		{
+			name:     "EmptyPath",
+			elements: []string{""},
+			expected: []string{""},
+		},
+		{
+			name:     "NilPath",
+			elements: nil,
+			// Gets "" back because individual elements are decoded and "" is the 0
+			// value for the decoder.
+			expected: []string{""},
+		},
+	}
+
+	for _, test := range table {
+		suite.Run(test.name, func() {
+			t := suite.T()
+			encoded := encodeAsPath(test.elements...)
+
+			// Sanity check, first and last character should not be '/'.
+			assert.Equal(t, strings.Trim(encoded, "/"), encoded)
+
+			decoded, err := decodePath(encoded)
+			require.NoError(t, err, clues.ToCore(err))
+
+			assert.Equal(t, test.expected, decoded)
+		})
+	}
+}
+
+func (suite *PathEncoderSuite) TestEncodeAsPathDecodePath_Error() {
+	t := suite.T()
+
+	inputElements := []string{
+		"some",
+		"path",
+	}
+
+	encoded := encodeAsPath(inputElements...)
+	// Randomly add an extra character outside the allowed character set which
+	// will mess up decoding the final element of the path.
+	encoded += "."
+
+	decoded, err := decodePath(encoded)
+	assert.Error(t, err)
+
+	for i := 0; i < len(inputElements)-1; i++ {
+		assert.Equal(t, inputElements[i], decoded[i], "path element at index %d", i)
+	}
+
+	splitEncoded := strings.Split(encoded, "/")
+
+	assert.Equal(
+		t,
+		splitEncoded[len(splitEncoded)-1],
+		decoded[len(decoded)-1],
+		"final path element that failed to decode")
+}
+
+func (suite *PathEncoderSuite) TestEncodeAsPathDecodeElements() {
 	table := []struct {
 		name     string
 		elements []string

src/internal/kopia/upload.go

@@ -2,7 +2,6 @@ package kopia
 
 import (
 	"context"
-	"encoding/base64"
 	"errors"
 	"runtime/trace"
 	"strings"
@@ -171,22 +170,16 @@ func (cp *corsoProgress) FinishedHashingFile(fname string, bs int64) {
 	// Pass the call through as well so we don't break expected functionality.
 	defer cp.UploadProgress.FinishedHashingFile(fname, bs)
 
-	sl := strings.Split(fname, "/")
-
-	for i := range sl {
-		rdt, err := base64.StdEncoding.DecodeString(sl[i])
-		if err != nil {
-			logger.Ctx(cp.ctx).Infow(
-				"unable to decode base64 path segment",
-				"segment", sl[i])
-		} else {
-			sl[i] = string(rdt)
-		}
+	decoded, err := decodePath(fname)
+	if err != nil {
+		logger.Ctx(cp.ctx).Infow(
+			"unable to decode base64 path elements",
+			"encoded_path", fname)
 	}
 
 	logger.Ctx(cp.ctx).Debugw(
 		"finished hashing file",
-		"path", clues.Hide(path.Elements(sl[2:])))
+		"path", clues.Hide(path.Elements(decoded)))
 
 	cp.counter.Add(count.PersistedHashedBytes, bs)
 	atomic.AddInt64(&cp.totalBytes, bs)