Fix int like cql types with negative range, add error when processing uint like go types
#306
Conversation
illia-li
force-pushed
the
il/fix/marshal/uints_overflow
branch
from
fa2b2a4 to
43a25cd
Compare
| case reflect.Uint, reflect.Uint64: | ||
| val := v.Uint() | ||
| if val > math.MaxInt64 { | ||
| return nil, fmt.Errorf("failed to marshal bigint: value (%T)(%[1]v) out of range", v.Interface()) | ||
| } | ||
| return encUint64(val), nil |
There was a problem hiding this comment.
why not just call EncUint64() here? there is the MaxInt64 check there so no need to duplicate
There was a problem hiding this comment.
To track exactly where the error occurred, some differences are needed in the errors.
In this case, these differences - the type of value (%T)(%[1]v).
If you do not do this, it will be more difficult to track the location of the error.
For me, if the content of all errors included the name of the function and something like a stack trace , this would be an ideal option.
| val := v.Uint() | ||
| if val > math.MaxInt64 { | ||
| return nil, fmt.Errorf("failed to marshal counter: value (%T)(%[1]v) out of range", v.Interface()) | ||
| } | ||
| return encUint64(val), nil |
There was a problem hiding this comment.
why not call EncUint64?
|
Please add the information about what are you fixing in the commit message |
|
@illia-li , @sylwiaszunejko , on some systems |
| @@ -398,7 +407,10 @@ func DecUintR(p []byte, v **uint) error { | |||
| *v = new(uint) | |||
| } | |||
| case 8: | |||
| val := uint(p[0])<<56 | uint(p[1])<<48 | uint(p[2])<<40 | uint(p[3])<<32 | uint(p[4])<<24 | uint(p[5])<<16 | uint(p[6])<<8 | uint(p[7]) | |||
| if p[0] > math.MaxInt8 { | |||
There was a problem hiding this comment.
Why do you consider only first byte ? It looks like mistake
There was a problem hiding this comment.
The p[0] > math.MaxInt8 condition is are like a question is a data value in the negative range?
|
I think we need first to come up with what is the "correct" behavior and then align code to it. |
All
intlikecql typeshavenegativerange andpositiverange, butuintlikego typeshave onlypositiverange.For example, if cast
-1tinyint[]byte("\xff")intouint likego typeswe got255.In the old marhsal/unmarshal functions (before the starts redesign), this problem was solved in different ways.
Now marhsal/unmarshal functions (before the starts redesign), allow the processing with a negative range of the
intlikecql typesfor alluint likego types.But this can lead to problems due to the different
DBandGOrepresentations of the same values .So, my suggest to leave only one way - stop the processing with a negative range of the
intlikecql typesfor alluint likego types.