improve ReadHandler

decoder.go

@@ -8,67 +8,18 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net/http"
-	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
 	"unsafe"
 )
 
-// ProtocolReadHandler defines a ReadFull interface.
+// ReadHandler defines a ReadFull interface.
 //
 // ReadFull should behaves as io.ReadFull in terms of reading the external resource.
 // The data already has the correct size so it can be used directly to store the read output.
-type ProtocolReadHandler interface {
-	ReadFull(scheme, uri string, data []byte) error
-}
-
-// ProtocolRegistry implements a secure ProtocolReadHandler supporting http, https and relative paths.
-// If Dir is empty the os.Getws will be used. It comes with directory traversal protection.
-// If HTTPClient is nil http[s] will not be supported.
-type ProtocolRegistry struct {
-	Dir        string
-	HTTPClient *http.Client
-}
-
-func (reg *ProtocolRegistry) readRelativeFile(uri string, data []byte) (err error) {
-	dir := reg.Dir
-	if dir == "" {
-		if dir, err = os.Getwd(); err != nil {
-			return
-		}
-	}
-	var f http.File
-	f, err = http.Dir(dir).Open(uri)
-	if err != nil {
-		return
-	}
-	_, err = io.ReadFull(f, data)
-	return
-}
-
-// ReadFull should as io.ReadFull in terms of reading the external resource.
-// An error is returned when the scheme is not supported.
-func (reg *ProtocolRegistry) ReadFull(scheme, uri string, data []byte) error {
-	switch scheme {
-	case "": // probably relative path
-		return reg.readRelativeFile(uri, data)
-	case "http://", "https://":
-		if reg.HTTPClient != nil {
-			resp, err := reg.HTTPClient.Get(uri)
-			if err != nil {
-				return err
-			}
-			defer resp.Body.Close()
-			if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-				return fmt.Errorf("gltf: server responded with %d", resp.StatusCode)
-			}
-			_, err = io.ReadFull(resp.Body, data)
-			return err
-		}
-	}
-	return errors.New("gltf: scheme not supported")
+type ReadHandler interface {
+	ReadFull(uri string, data []byte) error
 }
 
 // Open will open a glTF or GLB file specified by name and return the Document.
@@ -77,32 +28,41 @@ func Open(name string) (*Document, error) {
 	if err != nil {
 		return nil, err
 	}
+	defer f.Close()
+	dec := NewDecoder(f).WithReadHandler(&ProtocolRegistry{
+		"": &RelativeFileHandler{Dir: filepath.Dir(name)},
+	})
 	doc := new(Document)
-	err = NewDecoder(f).WithProtocolReadHandler(&ProtocolRegistry{Dir: filepath.Dir(name)}).Decode(doc)
-	f.Close()
+	if err = dec.Decode(doc); err != nil {
+		doc = nil
+	}
 	return doc, err
 }
 
 // A Decoder reads and decodes glTF and GLB values from an input stream.
-// Callback is called to read external resources.
-// If Callback is nil the external resource data in not loaded.
+// ReadHandler is called to read external resources.
 type Decoder struct {
-	ProtocolReadHandler ProtocolReadHandler
-	r                   *bufio.Reader
+	ReadHandler ReadHandler
+	r           *bufio.Reader
 }
 
 // NewDecoder returns a new decoder that reads from r.
-// By default the external buffers are not read.
+//
+// By default the external buffers in a relative path is supported.
+// The supported external buffer URIs can be easily extended using ProtocolRegistry
+// and adding custom handlers, such as for https:// or ftp://.
 func NewDecoder(r io.Reader) *Decoder {
 	return &Decoder{
-		ProtocolReadHandler: new(ProtocolRegistry),
-		r:                   bufio.NewReader(r),
+		r: bufio.NewReader(r),
+		ReadHandler: &ProtocolRegistry{
+			"": new(RelativeFileHandler),
+		},
 	}
 }
 
-// WithProtocolReadHandler sets the ProtocolReadHandler.
-func (d *Decoder) WithProtocolReadHandler(reg ProtocolReadHandler) *Decoder {
-	d.ProtocolReadHandler = reg
+// WithReadHandler sets the ReadHandler.
+func (d *Decoder) WithReadHandler(reg ReadHandler) *Decoder {
+	d.ReadHandler = reg
 	return d
 }
 
@@ -191,11 +151,10 @@ func (d *Decoder) decodeBuffer(buffer *Buffer) error {
 		buffer.Data, err = buffer.marshalData()
 	} else if err = validateBufferURI(buffer.URI); err == nil {
 		buffer.Data = make([]uint8, buffer.ByteLength)
-		var u *url.URL
-		u, err = url.Parse(buffer.URI)
-		if err == nil {
-			err = d.ProtocolReadHandler.ReadFull(u.Scheme, buffer.URI, buffer.Data)
-		}
+		err = d.ReadHandler.ReadFull(buffer.URI, buffer.Data)
+	}
+	if err != nil {
+		buffer.Data = nil
 	}
 	return err
 }

decoder_test.go

@@ -153,10 +153,13 @@ func (c *chunkedReader) Read(p []byte) (n int, err error) {
 	return 1, nil
 }
 
-func readCallback(payload string) func(string) (io.ReadCloser, error) {
-	return func(name string) (io.ReadCloser, error) {
-		return ioutil.NopCloser(&chunkedReader{s: []byte(payload)}), nil
-	}
+type mockReadHandler struct {
+	Payload string
+}
+
+func (m mockReadHandler) ReadFull(uri string, data []byte) error {
+	copy(data, []byte(m.Payload))
+	return nil
 }
 
 func TestDecoder_decodeBuffer(t *testing.T) {
@@ -174,7 +177,7 @@ func TestDecoder_decodeBuffer(t *testing.T) {
 		{"noURI", &Decoder{}, args{&Buffer{ByteLength: 1, URI: ""}}, nil, true},
 		{"invalidURI", &Decoder{}, args{&Buffer{ByteLength: 1, URI: "../a.bin"}}, nil, true},
 		{"noSchemeErr", NewDecoder(nil), args{&Buffer{ByteLength: 3, URI: "ftp://a.bin"}}, nil, true},
-		{"base", NewDecoder(nil).WithCallback(readCallback("abcdfg")), args{&Buffer{ByteLength: 6, URI: "a.bin"}}, []byte("abcdfg"), false},
+		{"base", NewDecoder(nil).WithReadHandler(&mockReadHandler{"abcdfg"}), args{&Buffer{ByteLength: 6, URI: "a.bin"}}, []byte("abcdfg"), false},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -233,9 +236,9 @@ func TestDecoder_Decode(t *testing.T) {
 		args    args
 		wantErr bool
 	}{
-		{"baseJSON", NewDecoder(bytes.NewBufferString("{\"buffers\": [{\"byteLength\": 1, \"URI\": \"a.bin\"}]}")).WithCallback(readCallback("abcdfg")), args{new(Document)}, false},
-		{"onlyGLBHeader", NewDecoder(bytes.NewBuffer([]byte{0x67, 0x6c, 0x54, 0x46, 0x02, 0x00, 0x00, 0x00, 0x40, 0x0b, 0x00, 0x00, 0x5c, 0x06, 0x00, 0x00, 0x4a, 0x53, 0x4f, 0x4e})).WithCallback(readCallback("abcdfg")), args{new(Document)}, true},
-		{"glbNoJSONChunk", NewDecoder(bytes.NewBuffer([]byte{0x67, 0x6c, 0x54, 0x46, 0x02, 0x00, 0x00, 0x00, 0x40, 0x0b, 0x00, 0x00, 0x5c, 0x06, 0x00, 0x00, 0x4a, 0x52, 0x4f, 0x4e})).WithCallback(readCallback("abcdfg")), args{new(Document)}, true},
+		{"baseJSON", NewDecoder(bytes.NewBufferString("{\"buffers\": [{\"byteLength\": 1, \"URI\": \"a.bin\"}]}")).WithReadHandler(&mockReadHandler{"abcdfg"}), args{new(Document)}, false},
+		{"onlyGLBHeader", NewDecoder(bytes.NewBuffer([]byte{0x67, 0x6c, 0x54, 0x46, 0x02, 0x00, 0x00, 0x00, 0x40, 0x0b, 0x00, 0x00, 0x5c, 0x06, 0x00, 0x00, 0x4a, 0x53, 0x4f, 0x4e})).WithReadHandler(&mockReadHandler{"abcdfg"}), args{new(Document)}, true},
+		{"glbNoJSONChunk", NewDecoder(bytes.NewBuffer([]byte{0x67, 0x6c, 0x54, 0x46, 0x02, 0x00, 0x00, 0x00, 0x40, 0x0b, 0x00, 0x00, 0x5c, 0x06, 0x00, 0x00, 0x4a, 0x52, 0x4f, 0x4e})).WithReadHandler(&mockReadHandler{"abcdfg"}), args{new(Document)}, true},
 		{"empty", NewDecoder(bytes.NewBufferString("")), args{new(Document)}, true},
 		{"invalidJSON", NewDecoder(bytes.NewBufferString("{asset: {}}")), args{new(Document)}, true},
 		{"invalidBuffer", NewDecoder(bytes.NewBufferString("{\"buffers\": [{\"byteLength\": 0}]}")), args{new(Document)}, true},

encode_test.go

@@ -3,13 +3,20 @@ package gltf
 import (
 	"bytes"
 	"fmt"
-	"io"
-	"io/ioutil"
 	"testing"
 
 	"github.com/go-test/deep"
 )
 
+type mockChunkReadHandler struct {
+	Chunks map[string][]byte
+}
+
+func (m mockChunkReadHandler) ReadFull(uri string, data []byte) error {
+	copy(data, m.Chunks[uri])
+	return nil
+}
+
 func saveMemory(doc *Document, asBinary bool) (*Decoder, error) {
 	buff := new(bytes.Buffer)
 	chunks := make(map[string][]byte)
@@ -23,13 +30,8 @@ func saveMemory(doc *Document, asBinary bool) (*Decoder, error) {
 	if err := e.Encode(doc); err != nil {
 		return nil, err
 	}
-	rcb := func(uri string) (io.ReadCloser, error) {
-		if chunk, ok := chunks[uri]; ok {
-			return ioutil.NopCloser(bytes.NewReader(chunk)), nil
-		}
-		return nil, nil
-	}
-	return NewDecoder(buff).WithCallback(rcb), nil
+
+	return NewDecoder(buff).WithReadHandler(&mockChunkReadHandler{chunks}), nil
 }
 
 func TestEncoder_Encode(t *testing.T) {
@@ -72,7 +74,7 @@ func TestEncoder_Encode(t *testing.T) {
 			{Extras: 8.0, Name: "binary", ByteLength: 3, URI: "a.bin", Data: []uint8{1, 2, 3}},
 			{Extras: 8.0, Name: "embedded", ByteLength: 2, URI: "data:application/octet-stream;base64,YW55ICsgb2xkICYgZGF0YQ==", Data: []byte("any + old & data")},
 			{Extras: 8.0, Name: "external", ByteLength: 4, URI: "b.bin", Data: []uint8{4, 5, 6, 7}},
-			{Extras: 8.0, Name: "external", ByteLength: 4, URI: "a.drc"},
+			{Extras: 8.0, Name: "external", ByteLength: 4, URI: "a.drc", Data: []uint8{0, 0, 0, 0}},
 		}}}, false},
 		{"withBufView", args{&Document{BufferViews: []BufferView{
 			{Extras: 8.0, Buffer: 0, ByteOffset: 1, ByteLength: 2, ByteStride: 5, Target: ArrayBuffer},

io.go

@@ -0,0 +1,48 @@
+package gltf
+
+import (
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"os"
+)
+
+// RelativeFileHandler implements a secure ReadHandler supporting relative paths.
+// If Dir is empty the os.Getws will be used. It comes with directory traversal protection.
+type RelativeFileHandler struct {
+	Dir string
+}
+
+// ReadFull should as io.ReadFull in terms of reading the external resource.
+func (h *RelativeFileHandler) ReadFull(uri string, data []byte) (err error) {
+	dir := h.Dir
+	if dir == "" {
+		if dir, err = os.Getwd(); err != nil {
+			return
+		}
+	}
+	var f http.File
+	f, err = http.Dir(dir).Open(uri)
+	if err != nil {
+		return
+	}
+	_, err = io.ReadFull(f, data)
+	return
+}
+
+// ProtocolRegistry implements a secure ProtocolReadHandler as a map of supported schemes.
+type ProtocolRegistry map[string]ReadHandler
+
+// ReadFull should as io.ReadFull in terms of reading the external resource.
+// An error is returned when the scheme is not supported.
+func (reg ProtocolRegistry) ReadFull(uri string, data []byte) error {
+	u, err := url.Parse(uri)
+	if err != nil {
+		return err
+	}
+	if f, ok := reg[u.Scheme]; ok {
+		return f.ReadFull(uri, data)
+	}
+	return errors.New("gltf: not supported scheme")
+}