feat(golang): array support and binary protocol for doubles #54
Conversation
kafka1991
commented
Aug 19, 2025
kafka1991
commented
- array support
- protocol version option, binary protocol for doubles
1. array support 2. protocol version option, binary protocol for doubles
|
Hi @sklarsa |
|
@puzpuzpuz Thanks for your review! |
| } | ||
|
|
||
| totalElements := product(shape) | ||
| if totalElements > MaxArrayElements { |
There was a problem hiding this comment.
Shouldn't we check the product for overflow?
There was a problem hiding this comment.
Example Attack:
// On 64-bit: uint max = 18,446,744,073,709,551,615
shape := []uint{4294967296, 4294967296} // Each within bounds individually
// product = 4294967296 * 4294967296 = 18446744073709551616 → OVERFLOWS to 0
// Bypasses MaxArrayElements check!
There was a problem hiding this comment.
fixed in 99d1a6c
I've asked if someone can configure that on this repo - I don't have the permissions. |
buffer.go
Outdated
| if dim1 > 0 { | ||
| dim2 = len(values[0]) | ||
| totalElements := dim1 * dim2 | ||
| if dim1 > MaxArrayElements || dim2 > MaxArrayElements || totalElements > MaxArrayElements || totalElements < 0 { |
There was a problem hiding this comment.
We need to improve overflow detection:
dim1 = 65536, dim2 = 65536
totalElements = 65536 * 65536 = 4294967296
On 32-bit systems: overflows to 0, bypasses validation
There was a problem hiding this comment.
fixed in 99d1a6c
|
@kafka1991 Vlad copied settings from the core repo to this one, but I still don't see the Approve and run CI button. Maybe a new commit needs to be pushed into your branch to have GH reevaluate the settings? |
The CI checks still haven't started. Maybe I should push to branch in the repo and reopen the PR. |
@kafka1991 if you have the permissions now, let's do this. |
|
Continue in a separate PR #55 |
2 participants
