Skip to content

Commit

Permalink
[internal][report] Fix bug in token parsing, add token JSON
Browse files Browse the repository at this point in the history
`internal/report.parseUploadToken` used to use `fmt.Sscanf`. It turns
out that this function succeeds even if it's only able to parse part
of its input, which means that a string with a prefix which is a
valid upload token was accepted. Fix this by replacing `fmt.Sscanf`
with `srcconv.ParseUint`.
  • Loading branch information
joshlf committed May 10, 2020
1 parent 8adff9c commit 855ce41
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 4 deletions.
32 changes: 28 additions & 4 deletions functions/internal/report/token.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package report

import (
"encoding/json"
"errors"
"fmt"
"strconv"
"strings"

"functions/internal/util"
)

// On the design of the upload token (see [1] for more details):
Expand Down Expand Up @@ -119,17 +123,37 @@ func (t UploadToken) String() string {
return string(scratch[:written-1])
}

var tokenParseError = errors.New("malformed upload token")
// MarshalJSON implements json.Marshaler.
func (t UploadToken) MarshalJSON() ([]byte, error) {
return json.Marshal(t.String())
}

// UnmarshalJSON implements json.Unmarshaler.
func (t *UploadToken) UnmarshalJSON(b []byte) error {
var s string
if err := json.Unmarshal(b, &s); err != nil {
return err
}
tt, err := parseUploadToken(s)
if err != nil {
return err
}

*t = tt
return nil
}

var tokenParseError = util.NewBadRequestError(errors.New("malformed upload token"))

func parseUploadToken(s string) (UploadToken, error) {
s = strings.ReplaceAll(s, "-", "")
if !strings.HasSuffix(s, "9") {
return UploadToken{}, tokenParseError
}

var t UploadToken
if _, err := fmt.Sscanf(s, "%o", &t.token); err != nil {
n, err := strconv.ParseUint(s[:len(s)-1], 8, 64)
if err != nil {
return UploadToken{}, tokenParseError
}
return t, nil
return UploadToken{token: n}, nil
}
10 changes: 10 additions & 0 deletions functions/internal/report/token_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package report

import (
"encoding/json"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -39,11 +40,20 @@ func TestTokenFormatParse(t *testing.T) {
// For each of the first 2^16 token values, ensure that parsing is the
// inverse of formatting.
for i := uint64(0); i < 1<<16; i++ {
// Test that formatting and parsing are inverses.
t0 := UploadToken{token: i}
s := t0.String()
t1, err := parseUploadToken(s)
assert.Nil(t, err)
assert.Equal(t, t0, t1)

// Test that JSON marshaling and unmarshaling are inverses.
bytes, err := json.Marshal(t0)
assert.Nil(t, err)
t1 = UploadToken{}
err = json.Unmarshal(bytes, &t1)
assert.Nil(t, err)
assert.Equal(t, t0, t1)
}
}

Expand Down

0 comments on commit 855ce41

Please sign in to comment.