Improve flate decompression logic and significantly improve memory usage #340
Conversation
| capHint := limit | ||
| if l <= limit/2 { | ||
| capHint = l * 2 | ||
| } | ||
| decompressed.Grow(capHint) |
There was a problem hiding this comment.
I believe this can be simplified to:
decompressed := bytes.NewBuffer(make([]byte, 0, min(limit, len(compressed) * 2)))There was a problem hiding this comment.
The if checking the l <= limit/2 protects against an int overflow on 32 bit devices if a packet is 1gb, which is impossible but having the if instead of min is technically correct
| // Handle no limit | ||
| if limit == math.MaxInt { | ||
| if _, err := io.Copy(&decompressed, r); err != nil { | ||
| return nil, fmt.Errorf("decompress flate: %w", err) | ||
| } | ||
| return decompressed.Bytes(), nil | ||
| } |
There was a problem hiding this comment.
Is there ever a sensible case to have no limit? I get that this is probably to avoid overflow, but could just do a max(limit + 1, math.MaxInt) somewhere.
There was a problem hiding this comment.
limit + 1 would cause an overflow so we cant handle this without checking limit == math.MaxInt, which is what the listener sets if the user configures no limit
gophertunnel/minecraft/listener.go
Line 133 in 8096f00
Sandertv
left a comment
There was a problem hiding this comment.
I think these changes look sensible. I assume this has been tested thoroughly?
Uh oh!
There was an error while loading. Please reload this page.