Merge pull request ipfs/go-ipfs-chunker#21 from ipfs/fix_rigorous_siz…

…ing_checks

Rigorous sizing checks

This commit was moved from ipfs/go-ipfs-chunker@72733e3

chunker/README.md

@@ -31,8 +31,6 @@ The package provides a `SizeSplitter` which creates chunks of equal size and it
 > go get github.com/ipfs/go-ipfs-chunker
 ```
 
-It uses [Gx](https://github.com/whyrusleeping/gx) to manage dependencies. You can use `make all` to build it with the `gx` dependencies.
-
 ## Usage
 
 ```

chunker/parse.go

@@ -8,9 +8,20 @@ import (
 	"strings"
 )
 
+const (
+	// DefaultBlockSize is the chunk size that splitters produce (or aim to).
+	DefaultBlockSize int64 = 1024 * 256
+
+	// No leaf block should contain more than 1MiB of payload data ( wrapping overhead aside )
+	// This effectively mandates the maximum chunk size
+	// See discussion at https://github.com/ipfs/go-ipfs-chunker/pull/21#discussion_r369124879 for background
+	ChunkSizeLimit int = 1048576
+)
+
 var (
 	ErrRabinMin = errors.New("rabin min must be greater than 16")
-	ErrSize     = errors.New("chunker size muster greater than 0")
+	ErrSize     = errors.New("chunker size must be greater than 0")
+	ErrSizeMax  = fmt.Errorf("chunker parameters may not exceed the maximum chunk size of %d", ChunkSizeLimit)
 )
 
 // FromString returns a Splitter depending on the given string:
@@ -28,6 +39,8 @@ func FromString(r io.Reader, chunker string) (Splitter, error) {
 			return nil, err
 		} else if size <= 0 {
 			return nil, ErrSize
+		} else if size > ChunkSizeLimit {
+			return nil, ErrSizeMax
 		}
 		return NewSizeSplitter(r, int64(size)), nil
 
@@ -51,6 +64,8 @@ func parseRabinString(r io.Reader, chunker string) (Splitter, error) {
 		size, err := strconv.Atoi(parts[1])
 		if err != nil {
 			return nil, err
+		} else if int(float32(size)*1.5) > ChunkSizeLimit { // FIXME - this will be addressed in a subsequent PR
+			return nil, ErrSizeMax
 		}
 		return NewRabin(r, uint64(size)), nil
 	case 4:
@@ -84,6 +99,14 @@ func parseRabinString(r io.Reader, chunker string) (Splitter, error) {
 			return nil, err
 		}
 
+		if min >= avg {
+			return nil, errors.New("incorrect format: rabin-min must be smaller than rabin-avg")
+		} else if avg >= max {
+			return nil, errors.New("incorrect format: rabin-avg must be smaller than rabin-max")
+		} else if max > ChunkSizeLimit {
+			return nil, ErrSizeMax
+		}
+
 		return NewRabinMinMax(r, uint64(min), uint64(avg), uint64(max)), nil
 	default:
 		return nil, errors.New("incorrect format (expected 'rabin' 'rabin-[avg]' or 'rabin-[min]-[avg]-[max]'")

chunker/parse_test.go

@@ -2,35 +2,79 @@ package chunk
 
 import (
 	"bytes"
+	"fmt"
 	"testing"
 )
 
+const (
+	testTwoThirdsOfChunkLimit = 2 * (float32(ChunkSizeLimit) / float32(3))
+)
+
 func TestParseRabin(t *testing.T) {
-	max := 1000
-	r := bytes.NewReader(randBuf(t, max))
-	chk1 := "rabin-18-25-32"
-	chk2 := "rabin-15-23-31"
-	_, err := parseRabinString(r, chk1)
+	r := bytes.NewReader(randBuf(t, 1000))
+
+	_, err := FromString(r, "rabin-18-25-32")
 	if err != nil {
 		t.Errorf(err.Error())
 	}
-	_, err = parseRabinString(r, chk2)
-	if err == ErrRabinMin {
-		t.Log("it should be ErrRabinMin here.")
+
+	_, err = FromString(r, "rabin-15-23-31")
+	if err != ErrRabinMin {
+		t.Fatalf("Expected an 'ErrRabinMin' error, got: %#v", err)
+	}
+
+	_, err = FromString(r, "rabin-20-20-21")
+	if err == nil || err.Error() != "incorrect format: rabin-min must be smaller than rabin-avg" {
+		t.Fatalf("Expected an arg-out-of-order error, got: %#v", err)
+	}
+
+	_, err = FromString(r, "rabin-19-21-21")
+	if err == nil || err.Error() != "incorrect format: rabin-avg must be smaller than rabin-max" {
+		t.Fatalf("Expected an arg-out-of-order error, got: %#v", err)
+	}
+
+	_, err = FromString(r, fmt.Sprintf("rabin-19-21-%d", ChunkSizeLimit))
+	if err != nil {
+		t.Fatalf("Expected success, got: %#v", err)
+	}
+
+	_, err = FromString(r, fmt.Sprintf("rabin-19-21-%d", 1+ChunkSizeLimit))
+	if err != ErrSizeMax {
+		t.Fatalf("Expected 'ErrSizeMax', got: %#v", err)
+	}
+
+	_, err = FromString(r, fmt.Sprintf("rabin-%.0f", testTwoThirdsOfChunkLimit))
+	if err != nil {
+		t.Fatalf("Expected success, got: %#v", err)
+	}
+
+	_, err = FromString(r, fmt.Sprintf("rabin-%.0f", 1+testTwoThirdsOfChunkLimit))
+	if err != ErrSizeMax {
+		t.Fatalf("Expected 'ErrSizeMax', got: %#v", err)
 	}
+
 }
 
 func TestParseSize(t *testing.T) {
-	max := 1000
-	r := bytes.NewReader(randBuf(t, max))
-	size1 := "size-0"
-	size2 := "size-32"
-	_, err := FromString(r, size1)
-	if err == ErrSize {
-		t.Log("it should be ErrSize here.")
-	}
-	_, err = FromString(r, size2)
-	if err == ErrSize {
-		t.Fatal(err)
+	r := bytes.NewReader(randBuf(t, 1000))
+
+	_, err := FromString(r, "size-0")
+	if err != ErrSize {
+		t.Fatalf("Expected an 'ErrSize' error, got: %#v", err)
+	}
+
+	_, err = FromString(r, "size-32")
+	if err != nil {
+		t.Fatalf("Expected success, got: %#v", err)
+	}
+
+	_, err = FromString(r, fmt.Sprintf("size-%d", ChunkSizeLimit))
+	if err != nil {
+		t.Fatalf("Expected success, got: %#v", err)
+	}
+
+	_, err = FromString(r, fmt.Sprintf("size-%d", 1+ChunkSizeLimit))
+	if err != ErrSizeMax {
+		t.Fatalf("Expected 'ErrSizeMax', got: %#v", err)
 	}
 }

chunker/splitting.go

@@ -13,9 +13,6 @@ import (
 
 var log = logging.Logger("chunk")
 
-// DefaultBlockSize is the chunk size that splitters produce (or aim to).
-var DefaultBlockSize int64 = 1024 * 256
-
 // A Splitter reads bytes from a Reader and creates "chunks" (byte slices)
 // that can be used to build DAG nodes.
 type Splitter interface {