Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for k8s bytes suffices #53

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 112 additions & 34 deletions decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,8 @@
// Conversion through JSON/YAML may have converted integers into floats, including
// exponential notation. This function will parse those values back into integers
// if possible.
func parseUintLiteral(value string) (uint64, error) {

Check failure on line 467 in decode.go

View workflow job for this annotation

GitHub Actions / ci (1.23.x)

cyclomatic complexity 17 of func `parseUintLiteral` is high (> 15) (gocyclo)
// Try to parse as an unsigned integer.
base := 10
if len(value) >= 2 && strings.HasPrefix(value, "0") {
switch value[1] {
Expand All @@ -479,20 +480,39 @@
value = value[2:]
}
}

parsed, err := strconv.ParseUint(value, base, 64)
if err != nil {
parsedFloat, floatErr := strconv.ParseFloat(value, 64)
if floatErr != nil || parsedFloat < 0 || math.IsInf(parsedFloat, 0) || math.IsNaN(parsedFloat) {
if err == nil || base != 10 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want to support hex literals as well if this fails? https://go.dev/ref/spec#Floating-point_literals

return parsed, err
}

// Try to parse as an unsigned integer encoded as a float.
parsedFloat, floatErr := strconv.ParseFloat(value, 64)
if floatErr == nil {
if parsedFloat < 0 || math.IsInf(parsedFloat, 0) || math.IsNaN(parsedFloat) {
return 0, err
}

// See if it's actually an integer.
parsed = uint64(parsedFloat)
if float64(parsed) != parsedFloat || parsed >= (1<<53) {
return parsed, errors.New("precision loss")
}
return parsed, nil
}

// Try to parse as bytes.
byteCount := &big.Int{}
rem, bytesErr := parseBytes(value, byteCount)
switch {
case bytesErr != nil:
return 0, bytesErr // Likely a better error message.
case rem != "": // Extra characters.
return 0, err
}
return parsed, nil
if byteCount.BitLen() > 64 {
return 0, errors.New("integer is too large")
}
return byteCount.Uint64(), nil
}

type intLit struct {
Expand Down Expand Up @@ -1225,8 +1245,11 @@
// nanosPerSecond is the number of nanoseconds in a second.
var nanosPerSecond = new(big.Int).SetUint64(uint64(time.Second / time.Nanosecond))

// nanosMap is a map of time unit names to their duration in nanoseconds.
var nanosMap = map[string]*big.Int{
// durationUnitNames is the (normalized) list of time unit names.
var durationUnitNames = []string{"h", "m", "s", "ms", "us", "ns"}

// nanosPerUnitMap is a map of time unit names to their duration in nanoseconds.
var nanosPerUnitMap = map[string]*big.Int{
"ns": new(big.Int).SetUint64(1), // Identity for nanos.
"us": new(big.Int).SetUint64(uint64(time.Microsecond / time.Nanosecond)),
"µs": new(big.Int).SetUint64(uint64(time.Microsecond / time.Nanosecond)), // U+00B5 = micro symbol
Expand All @@ -1237,41 +1260,20 @@
"h": new(big.Int).SetUint64(uint64(time.Hour / time.Nanosecond)),
}

// unitsNames is the (normalized) list of time unit names.
var unitsNames = []string{"h", "m", "s", "ms", "us", "ns"}

// parseDurationNest parses a single segment of the duration string.
func parseDurationNext(str string, totalNanos *big.Int) (string, error) {
// The next character must be [0-9.]
if !(str[0] == '.' || '0' <= str[0] && str[0] <= '9') {
return "", errors.New("invalid duration")
}
var err error
var whole, frac uint64
var pre bool // Whether we have seen a digit before the dot.
whole, str, pre, err = leadingInt(str)
// Parse the number and unit.
whole, frac, scale, unitName, str, err := parseNumberWithUnit(str)
if err != nil {
return "", err
}
var scale *big.Int
var post bool // Whether we have seen a digit after the dot.
if str != "" && str[0] == '.' {
str = str[1:]
frac, scale, str, post = leadingFrac(str)
}
if !pre && !post {
return "", errors.New("invalid duration")
if unitName == "" {
return "", fmt.Errorf("invalid duration: missing unit, expected one of %v", durationUnitNames)
}

end := unitEnd(str)
if end == 0 {
return "", fmt.Errorf("invalid duration: missing unit, expected one of %v", unitsNames)
}
unitName := str[:end]
str = str[end:]
nanosPerUnit, ok := nanosMap[unitName]
nanosPerUnit, ok := nanosPerUnitMap[unitName]
if !ok {
return "", fmt.Errorf("invalid duration: unknown unit, expected one of %v", unitsNames)
return "", fmt.Errorf("invalid duration: unknown unit, expected one of %v", durationUnitNames)
}

// Convert to nanos and add to total.
Expand All @@ -1296,6 +1298,82 @@
return str, nil
}

// bytesUnitNames is the list of byte unit names.
// E is exabyte, P is petabyte, T is terabyte, G is gigabyte, M is megabyte, K is kilobyte.
// The i suffix indicates a binary unit (e.g., Ki = 1024).
var bytesUnitNames = []string{"K", "M", "G", "T", "P", "E", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var bytesUnitNames = []string{"K", "M", "G", "T", "P", "E", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei"}
var bytesUnitNames = []string{"k", "M", "G", "T", "P", "E", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei"}


// A map from unit to the number of bytes in that unit.
var bytesPerUnitMap = map[string]int64{
"K": 1000,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Ki": 1 << 10,
"M": 1000 * 1000,
"Mi": 1 << 20,
"G": 1000 * 1000 * 1000,
"Gi": 1 << 30,
"T": 1000 * 1000 * 1000 * 1000,
"Ti": 1 << 40,
"P": 1000 * 1000 * 1000 * 1000 * 1000,
"Pi": 1 << 50,
"E": 1000 * 1000 * 1000 * 1000 * 1000 * 1000,
"Ei": 1 << 60,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

k8s also supports m (milli): https://github.com/kubernetes/kubernetes/blob/ed9572d9c7733602de43979caf886fd4092a7b0f/staging/src/k8s.io/apimachinery/pkg/api/resource/suffix.go#L96-L97

This doesn't make sense for bytes but if we want to support CPU units this could be useful.


func parseBytes(str string, totalBytes *big.Int) (string, error) {
whole, frac, scale, unitName, str, err := parseNumberWithUnit(str)
if err != nil {
return "", err
}

bytesPerUnit, ok := bytesPerUnitMap[unitName]
if !ok {
return "", fmt.Errorf("invalid bytes: unknown unit, expected one of %v", bytesUnitNames)
}

// Convert to bytes and add to total.
// totalBytes += whole * bytesPerUnit + frac * bytesPerUnit / scale
if whole > 0 {
wholeBytes := &big.Int{}
wholeBytes.SetUint64(whole)
wholeBytes.Mul(wholeBytes, big.NewInt(bytesPerUnit))
totalBytes.Add(totalBytes, wholeBytes)
}
if frac > 0 {
fracBytes := &big.Int{}
fracBytes.SetUint64(frac)
fracBytes.Mul(fracBytes, big.NewInt(bytesPerUnit))
rem := &big.Int{}
fracBytes.QuoRem(fracBytes, scale, rem)
totalBytes.Add(totalBytes, fracBytes)
}
return str, nil
}

func parseNumberWithUnit(str string) (whole, frac uint64, scale *big.Int, unitName string, rem string, err error) {
// The next character must be [0-9.]
if !(str[0] == '.' || '0' <= str[0] && str[0] <= '9') {
Copy link
Member

@pkwarren pkwarren Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we'll panic here on an empty string - verified by adding this to the test suite:

  - single_fixed64: ""

return whole, frac, scale, unitName, str, errors.New("invalid number, expected digit")
}
var pre bool // Whether we have seen a digit before the dot.
whole, str, pre, err = leadingInt(str)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to enforce a max length on the str somewhere so we don't read arbitrarily large bigint values and perform computations on them?

if err != nil {
return whole, frac, scale, unitName, str, err
}
var post bool // Whether we have seen a digit after the dot.
if str != "" && str[0] == '.' {
str = str[1:]
frac, scale, str, post = leadingFrac(str)
}
if !pre && !post {
return whole, frac, scale, unitName, str, errors.New("invalid number, expected digit")
}

end := unitEnd(str)
unitName = str[:end]
str = str[end:]
return whole, frac, scale, unitName, str, nil
}

func unitEnd(str string) int {
var i int
for ; i < len(str); i++ {
Expand Down
16 changes: 8 additions & 8 deletions decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,17 @@ func TestParseDuration(t *testing.T) {
}{
{Input: "", Expected: nil, ErrMsg: "invalid duration"},
{Input: "-", Expected: nil, ErrMsg: "invalid duration"},
{Input: "s", Expected: nil, ErrMsg: "invalid duration"},
{Input: ".", Expected: nil, ErrMsg: "invalid duration"},
{Input: "-s", Expected: nil, ErrMsg: "invalid duration"},
{Input: ".s", Expected: nil, ErrMsg: "invalid duration"},
{Input: "-.", Expected: nil, ErrMsg: "invalid duration"},
{Input: "-.s", Expected: nil, ErrMsg: "invalid duration"},
{Input: "--0s", Expected: nil, ErrMsg: "invalid duration"},
{Input: "s", Expected: nil, ErrMsg: "invalid number"},
{Input: ".", Expected: nil, ErrMsg: "invalid number"},
{Input: "-s", Expected: nil, ErrMsg: "invalid number"},
{Input: ".s", Expected: nil, ErrMsg: "invalid number"},
{Input: "-.", Expected: nil, ErrMsg: "invalid number"},
{Input: "-.s", Expected: nil, ErrMsg: "invalid number"},
{Input: "--0s", Expected: nil, ErrMsg: "invalid number"},
{Input: "0y", Expected: nil, ErrMsg: "unknown unit"},
{Input: "0so", Expected: nil, ErrMsg: "unknown unit"},
{Input: "0os", Expected: nil, ErrMsg: "unknown unit"},
{Input: "0s-0ms", Expected: nil, ErrMsg: "invalid duration"},
{Input: "0s-0ms", Expected: nil, ErrMsg: "invalid number"},
{Input: "0.5ns", Expected: nil, ErrMsg: "fractional nanos"},
{Input: "0.0005us", Expected: nil, ErrMsg: "fractional nanos"},
{Input: "0.0000005μs", Expected: nil, ErrMsg: "fractional nanos"},
Expand Down
34 changes: 27 additions & 7 deletions internal/testdata/basic.proto3test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -102,23 +102,23 @@ internal/testdata/basic.proto3test.yaml:52:19 invalid integer: strconv.ParseUint
52 | - single_int32: --1
52 | ..................^

internal/testdata/basic.proto3test.yaml:53:19 invalid integer: strconv.ParseUint: parsing "--1": invalid syntax
internal/testdata/basic.proto3test.yaml:53:19 invalid integer: invalid number, expected digit
53 | - single_int32: ---1
53 | ..................^

internal/testdata/basic.proto3test.yaml:54:19 invalid integer: strconv.ParseUint: parsing "inf": invalid syntax
54 | - single_int32: inf
54 | ..................^

internal/testdata/basic.proto3test.yaml:55:19 invalid integer: strconv.ParseUint: parsing ".inf": invalid syntax
internal/testdata/basic.proto3test.yaml:55:19 invalid integer: invalid number, expected digit
55 | - single_int32: .inf
55 | ..................^

internal/testdata/basic.proto3test.yaml:56:19 invalid integer: strconv.ParseUint: parsing ".inf": invalid syntax
internal/testdata/basic.proto3test.yaml:56:19 invalid integer: invalid number, expected digit
56 | - single_int32: -.inf
56 | ..................^

internal/testdata/basic.proto3test.yaml:57:19 invalid integer: strconv.ParseUint: parsing ".nan": invalid syntax
internal/testdata/basic.proto3test.yaml:57:19 invalid integer: invalid number, expected digit
57 | - single_int32: .nan
57 | ..................^

Expand Down Expand Up @@ -162,7 +162,7 @@ internal/testdata/basic.proto3test.yaml:98:21 expected sequence, got scalar
98 | - repeated_int32: 1
98 | ....................^

internal/testdata/basic.proto3test.yaml:100:30 invalid integer: strconv.ParseUint: parsing "hi": invalid syntax
internal/testdata/basic.proto3test.yaml:100:30 invalid integer: invalid number, expected digit
100 | - repeated_int32: [1, "1", hi]
100 | .............................^

Expand Down Expand Up @@ -194,11 +194,11 @@ internal/testdata/basic.proto3test.yaml:110:22 invalid duration: fractional nano
110 | - single_duration: 1.0123456789s
110 | .....................^

internal/testdata/basic.proto3test.yaml:111:22 invalid duration
internal/testdata/basic.proto3test.yaml:111:22 invalid number, expected digit
111 | - single_duration: As
111 | .....................^

internal/testdata/basic.proto3test.yaml:112:22 invalid duration
internal/testdata/basic.proto3test.yaml:112:22 invalid number, expected digit
112 | - single_duration: A.1s
112 | .....................^

Expand Down Expand Up @@ -233,3 +233,23 @@ internal/testdata/basic.proto3test.yaml:130:23 expected fields for google.protob
internal/testdata/basic.proto3test.yaml:135:7 unknown field "@type", expected one of [value]
135 | "@type": type.googleapis.com/google.protobuf.Int32Value
135 | ......^

internal/testdata/basic.proto3test.yaml:142:21 invalid integer: integer is too large
142 | - single_fixed64: 1000Ei
142 | ....................^

internal/testdata/basic.proto3test.yaml:144:19 integer is too small: < -2147483648
144 | - single_int32: -1Ti
144 | ..................^

internal/testdata/basic.proto3test.yaml:145:19 invalid integer: invalid bytes: unknown unit, expected one of [K M G T P E Ki Mi Gi Ti Pi Ei]
145 | - single_int32: 1Ai
145 | ..................^

internal/testdata/basic.proto3test.yaml:146:19 invalid integer: precision loss
146 | - single_int32: 1.1
146 | ..................^

internal/testdata/basic.proto3test.yaml:147:19 invalid integer: strconv.ParseUint: parsing "1Mi1K": invalid syntax
147 | - single_int32: 1Mi1K
147 | ..................^
9 changes: 9 additions & 0 deletions internal/testdata/basic.proto3test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,12 @@ values:
value: 1
- single_bytes: "nopad+"
- single_bytes: "web-safe__"
- single_fixed64: 1E
- single_fixed64: 1.0Mi
- single_fixed64: 1.1Gi
- single_fixed64: 1000Ei
- single_int32: -1Ki
- single_int32: -1Ti
- single_int32: 1Ai
- single_int32: 1.1
- single_int32: 1Mi1K
4 changes: 2 additions & 2 deletions internal/testdata/dynamic.const.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ internal/testdata/dynamic.const.yaml:16:12 expected bool, got "null"
16 | value: null
16 | ...........^

internal/testdata/dynamic.const.yaml:19:12 invalid integer: strconv.ParseUint: parsing "true": invalid syntax
internal/testdata/dynamic.const.yaml:19:12 invalid integer: invalid number, expected digit
19 | value: true
19 | ...........^

Expand All @@ -34,7 +34,7 @@ internal/testdata/dynamic.const.yaml:46:30 invalid integer: precision loss
46 | repeated_int32: [1, "1", 1.5, hi, Infinity, NaN]
46 | .............................^

internal/testdata/dynamic.const.yaml:46:35 invalid integer: strconv.ParseUint: parsing "hi": invalid syntax
internal/testdata/dynamic.const.yaml:46:35 invalid integer: invalid number, expected digit
46 | repeated_int32: [1, "1", 1.5, hi, Infinity, NaN]
46 | ..................................^

Expand Down
Loading