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

Unexpected results with padded strings #182

Open
jsumners opened this issue Apr 1, 2023 · 3 comments
Open

Unexpected results with padded strings #182

jsumners opened this issue Apr 1, 2023 · 3 comments

Comments

@jsumners
Copy link

jsumners commented Apr 1, 2023

https://play.golang.com/p/3oTZVLEkdas

package main

import (
	"fmt"
	"github.com/spf13/cast"
)

func main() {
	x := cast.ToInt("08")
	fmt.Printf("08 => %d\n", x)
	
	x = cast.ToInt("8")
	fmt.Printf("8 => %d\n", x)
}

I'd expect the result to be 8 in each scenario. However, "08" ends up hitting a path where the base is changed and the parsing returns an error that we don't see.

@shplume
Copy link

shplume commented Apr 5, 2023

By reading the source code, I found that this problem occurs because 0 is the prefix of octal (just like 0x for hex). Maybe you can discard the leading 0 first.

@jsumners
Copy link
Author

jsumners commented Apr 5, 2023

Yes, that is what I stated in the original report. The path is

  1. cast/caste.go

    Lines 491 to 496 in ba0a5b6

    case string:
    v, err := strconv.ParseInt(trimZeroDecimal(s), 0, 0)
    if err == nil {
    return int(v), nil
    }
    return 0, fmt.Errorf("unable to cast %#v of type %T to int64", i, i)
  2. https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/strconv/atoi.go;l=202
  3. https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/strconv/atoi.go;l=78
  4. https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/strconv/atoi.go;l=107
  5. https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/strconv/atoi.go;l=131
  6. https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/strconv/atoi.go;l=145
  7. https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/strconv/atoi.go;l=153
  8. https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/strconv/atoi.go;l=53
  9. https://cs.opensource.google/go/go/+/refs/tags/go1.20.3:src/strconv/atoi.go;l=225
  10. cast/caste.go

    Line 496 in ba0a5b6

    return 0, fmt.Errorf("unable to cast %#v of type %T to int64", i, i)

The point is, cast.ToInt should consider leading 0 characters and do the right thing. Basically, I would only support 0o (0O) to indicate octals when casting (https://go.dev/ref/spec#Integer_literals).

@shplume
Copy link

shplume commented Apr 5, 2023

https://play.golang.com/p/mPCpDwylr7T

Thank you for the explanation. In my opinion, if cast.ToInt considers leading 0 characters, cast.ToInt("012") will return 12, but int variables with value 012 is 10 in Go(like above). I think this might not be appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants