Skip to content

Commit

Permalink
Fix control-byte computation at boundary sizes
Browse files Browse the repository at this point in the history
`writeCtrlByte` writes the left-over value using:

   byte((leftOver >> (8 * i)) & 0xFF)

This means that, after the `leftOver` value has been shifted, it should have a value in the [0,255] range so that the bitmask operation can work properly.

Currently, because the switch case determining `leftOver` is inclusive (<=) rather than inclusive (<), the shifted `leftOver` value can reach the value of 256, making an MMDB file invalid.
  • Loading branch information
max-ipinfo authored Sep 26, 2023
1 parent 561ff78 commit 7932ea8
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 4 deletions.
8 changes: 4 additions & 4 deletions mmdbtype/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -723,23 +723,23 @@ func writeCtrlByte(w writer, t DataType) (int64, error) {
switch {
case size < firstSize:
firstByte |= byte(size)
case size <= secondSize:
case size < secondSize:
firstByte |= 29
leftOver = size - firstSize
leftOverSize = 1
case size <= thirdSize:
case size < thirdSize:
firstByte |= 30
leftOver = size - secondSize
leftOverSize = 2
case size <= maxSize:
case size < maxSize:
firstByte |= 31
leftOver = size - thirdSize
leftOverSize = 3
default:
return 0, fmt.Errorf(
"cannot store %d bytes; max size is %d",
size,
maxSize,
maxSize-1,
)
}

Expand Down
51 changes: 51 additions & 0 deletions tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"math/big"
"net"
"os"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -110,6 +111,36 @@ func TestTreeInsertAndGet(t *testing.T) {
"utf8_string": "unicode! ☯ - ♫",
}

stringsGetRecord := mmdbtype.Map{
// firstSize
"size28": mmdbtype.String(strings.Repeat("*", 28)),
"size29": mmdbtype.String(strings.Repeat("*", 29)),
"size30": mmdbtype.String(strings.Repeat("*", 30)),
// secondSize
"size284": mmdbtype.String(strings.Repeat("*", 284)),
"size285": mmdbtype.String(strings.Repeat("*", 285)),
"size286": mmdbtype.String(strings.Repeat("*", 286)),
// thirdSize
"size65820": mmdbtype.String(strings.Repeat("*", 65820)),
"size65821": mmdbtype.String(strings.Repeat("*", 65821)),
"size65822": mmdbtype.String(strings.Repeat("*", 65822)),
// maxSize
"maxSizeMinus1": mmdbtype.String(strings.Repeat("*", 16843036)),
}

var stringsLookupRecord any = map[string]any{
"size28": strings.Repeat("*", 28),
"size29": strings.Repeat("*", 29),
"size30": strings.Repeat("*", 30),
"size284": strings.Repeat("*", 284),
"size285": strings.Repeat("*", 285),
"size286": strings.Repeat("*", 286),
"size65820": strings.Repeat("*", 65820),
"size65821": strings.Repeat("*", 65821),
"size65822": strings.Repeat("*", 65822),
"maxSizeMinus1": strings.Repeat("*", 16843036),
}

tests := []struct {
name string
disableIPv4Aliasing bool
Expand Down Expand Up @@ -536,6 +567,26 @@ func TestTreeInsertAndGet(t *testing.T) {
},
expectedNodeCount: 375,
},
{
name: "insertion of strings at boundary control byte size",
inserts: []testInsert{
{
network: "1.1.1.1/32",
start: "1.1.1.1",
end: "1.1.1.1",
value: stringsGetRecord,
},
},
gets: []testGet{
{
ip: "1.1.1.1",
expectedNetwork: "1.1.1.1/32",
expectedGetValue: stringsGetRecord,
expectedLookupValue: &stringsLookupRecord,
},
},
expectedNodeCount: 375,
},
}

for _, recordSize := range []int{24, 28, 32} {
Expand Down

0 comments on commit 7932ea8

Please sign in to comment.