chriscasola commented on code in PR #13381:
URL: https://github.com/apache/arrow/pull/13381#discussion_r897331203
##########
go/arrow/array/string.go:
##########
@@ -113,6 +113,12 @@ func (a *String) setData(data *Data) {
if offsets := data.buffers[1]; offsets != nil {
a.offsets = arrow.Int32Traits.CastFromBytes(offsets.Bytes())
}
+
+ expNumOffsets := a.array.data.offset + a.array.data.length + 1
+ if a.array.data.length > 0 &&
+ (len(a.offsets) < expNumOffsets ||
int(a.offsets[expNumOffsets-2]) > len(a.values)) {
+ panic("arrow/array: string offsets out of bounds of data
buffer")
+ }
Review Comment:
Are you okay with the `Concatenate` error becoming a panic?
I can remove the logic that allows the last offset to overflow and apply
this fix to binary arrays as well.
Also, I said I would change `int(a.offsets[expNumOffsets-2]) >
len(a.values)` to `int(a.offsets[expNumOffsets-2]) >= len(a.values)` but that
doesn't work if the last value in the string array is empty string. In that
case the last offset will be equal to `len(a.values)`, so I'm going to leave it
as I had it.
> If we allow the last offset to go past the end of the data slice, it
doesn't solve the original panic you were trying to prevent in the first place
as calling ValueBytes would still result in an out of bounds panic in that
situation.
It actually does still solve my original panic. My original issue would be
caught because the second-to-last offset was also past the end.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
