Repository: qmuntal/gltf
Pull request #13 (2019-05-09 16:03:40)

Improve Read Handler API
External resources are read using:
```go
type ReadHandler interface {
	ReadFull(uri string, data []byte) error
}
```

Two implementations provides: ProtocolRegistry and RelativeFileHandler


Patch:
diff --git a/decoder.go b/decoder.go
index f038dbb..d492965 100644
--- a/decoder.go
+++ b/decoder.go
@@ -14,13 +14,12 @@ import (
 	"unsafe"
 )
 
-// ReadResourceCallback defines a callback that will be called when an external resource should be loaded.
-// The string parameter is the URI of the resource.
-// If the reader and the error are nil the buffer data won't be loaded into memory.
-type ReadResourceCallback = func(string) (io.ReadCloser, error)
-
-func nilReadData(uri string) (io.ReadCloser, error) {
-	return nil, nil
+// 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 ReadHandler interface {
+	ReadFull(uri string, data []byte) error
 }
 
 // Open will open a glTF or GLB file specified by name and return the Document.
@@ -29,35 +28,41 @@ func Open(name string) (*Document, error) {
 	if err != nil {
 		return nil, err
 	}
-	cb := func(uri string) (io.ReadCloser, error) {
-		return os.Open(filepath.Join(filepath.Dir(name), uri))
-	}
+	defer f.Close()
+	dec := NewDecoder(f).WithReadHandler(&ProtocolRegistry{
+		"": &RelativeFileHandler{Dir: filepath.Dir(name)},
+	})
 	doc := new(Document)
-	err = NewDecoder(f).WithCallback(cb).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 {
-	Callback ReadResourceCallback
-	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{
-		Callback: nilReadData,
-		r:        bufio.NewReader(r),
+		r: bufio.NewReader(r),
+		ReadHandler: &ProtocolRegistry{
+			"": new(RelativeFileHandler),
+		},
 	}
 }
 
-// WithCallback sets the ReadResourceCallback.
-func (d *Decoder) WithCallback(c ReadResourceCallback) *Decoder {
-	d.Callback = c
+// WithReadHandler sets the ReadHandler.
+func (d *Decoder) WithReadHandler(reg ReadHandler) *Decoder {
+	d.ReadHandler = reg
 	return d
 }
 
@@ -142,16 +147,14 @@ func (d *Decoder) decodeBuffer(buffer *Buffer) error {
 		return errors.New("gltf: buffer without URI")
 	}
 	var err error
-	var r io.ReadCloser
 	if buffer.IsEmbeddedResource() {
 		buffer.Data, err = buffer.marshalData()
 	} else if err = validateBufferURI(buffer.URI); err == nil {
-		r, err = d.Callback(buffer.URI)
-		if r != nil && err == nil {
-			buffer.Data = make([]uint8, buffer.ByteLength)
-			_, err = io.ReadFull(r, buffer.Data)
-			r.Close()
-		}
+		buffer.Data = make([]uint8, buffer.ByteLength)
+		err = d.ReadHandler.ReadFull(buffer.URI, buffer.Data)
+	}
+	if err != nil {
+		buffer.Data = nil
 	}
 	return err
 }
diff --git a/encode.go b/encode.go
index 66aa398..23c27da 100644
--- a/encode.go
+++ b/encode.go
@@ -22,7 +22,7 @@ func Save(doc *Document, name string) error {
 	return save(doc, name, false)
 }
 
-// Save will save a document as a GLB file with the specified by name.
+// SaveBinary will save a document as a GLB file with the specified by name.
 func SaveBinary(doc *Document, name string) error {
 	return save(doc, name, true)
 }
diff --git a/io.go b/io.go
new file mode 100644
index 0000000..2126a08
--- /dev/null
+++ b/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")
+}


Test patch:
diff --git a/decoder_test.go b/decoder_test.go
index da69edb..1f327f8 100644
--- a/decoder_test.go
+++ b/decoder_test.go
@@ -2,7 +2,6 @@ package gltf
 
 import (
 	"bytes"
-	"errors"
 	"io"
 	"io/ioutil"
 	"reflect"
@@ -154,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,9 +176,8 @@ func TestDecoder_decodeBuffer(t *testing.T) {
 		{"byteLength_0", &Decoder{}, args{&Buffer{ByteLength: 0, URI: "a.bin"}}, nil, true},
 		{"noURI", &Decoder{}, args{&Buffer{ByteLength: 1, URI: ""}}, nil, true},
 		{"invalidURI", &Decoder{}, args{&Buffer{ByteLength: 1, URI: "../a.bin"}}, nil, true},
-		{"cbErr", NewDecoder(nil).WithCallback(func(name string) (io.ReadCloser, error) { return nil, errors.New("") }), args{&Buffer{ByteLength: 3, URI: "a.bin"}}, nil, true},
-		{"noFilBuf", NewDecoder(nil).WithCallback(readCallback("")), args{&Buffer{ByteLength: 30, URI: "a.bin"}}, make([]byte, 30), true},
-		{"base", NewDecoder(nil).WithCallback(readCallback("abcdfg")), args{&Buffer{ByteLength: 6, URI: "a.bin"}}, []byte("abcdfg"), false},
+		{"noSchemeErr", NewDecoder(nil), args{&Buffer{ByteLength: 3, URI: "ftp://a.bin"}}, nil, true},
+		{"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) {
@@ -235,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},
diff --git a/encode_test.go b/encode_test.go
index 24b4fe5..3464b88 100644
--- a/encode_test.go
+++ b/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},
diff --git a/io_test.go b/io_test.go
new file mode 100644
index 0000000..0933413
--- /dev/null
+++ b/io_test.go
@@ -0,0 +1,47 @@
+package gltf
+
+import "testing"
+
+func TestRelativeFileHandler_ReadFull(t *testing.T) {
+	type args struct {
+		uri  string
+		data []byte
+	}
+	tests := []struct {
+		name    string
+		h       *RelativeFileHandler
+		args    args
+		wantErr bool
+	}{
+		{"no dir", new(RelativeFileHandler), args{"a.bin", []byte{}}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := tt.h.ReadFull(tt.args.uri, tt.args.data); (err != nil) != tt.wantErr {
+				t.Errorf("RelativeFileHandler.ReadFull() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestProtocolRegistry_ReadFull(t *testing.T) {
+	type args struct {
+		uri  string
+		data []byte
+	}
+	tests := []struct {
+		name    string
+		reg     ProtocolRegistry
+		args    args
+		wantErr bool
+	}{
+		{"invalid url", make(ProtocolRegistry), args{"%$·$·23", []byte{}}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := tt.reg.ReadFull(tt.args.uri, tt.args.data); (err != nil) != tt.wantErr {
+				t.Errorf("ProtocolRegistry.ReadFull() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}