From 36db5bf63c094307e8ed6ed6af55c2093635917a Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 26 Sep 2019 12:26:10 -0700 Subject: [PATCH 1/9] dep: update go-ipfs-files/go-unixfs Fixes symlink sizes. --- go.mod | 4 ++-- go.sum | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 6d59fd7dbe9..4e471b7193b 100644 --- a/go.mod +++ b/go.mod @@ -34,7 +34,7 @@ require ( github.com/ipfs/go-ipfs-ds-help v0.0.1 github.com/ipfs/go-ipfs-exchange-interface v0.0.1 github.com/ipfs/go-ipfs-exchange-offline v0.0.1 - github.com/ipfs/go-ipfs-files v0.0.4 + github.com/ipfs/go-ipfs-files v0.0.6 github.com/ipfs/go-ipfs-pinner v0.0.3 github.com/ipfs/go-ipfs-posinfo v0.0.1 github.com/ipfs/go-ipfs-provider v0.3.0 @@ -50,7 +50,7 @@ require ( github.com/ipfs/go-metrics-prometheus v0.0.2 github.com/ipfs/go-mfs v0.1.1 github.com/ipfs/go-path v0.0.7 - github.com/ipfs/go-unixfs v0.2.1 + github.com/ipfs/go-unixfs v0.2.3 github.com/ipfs/go-verifcid v0.0.1 github.com/ipfs/interface-go-ipfs-core v0.2.5 github.com/jbenet/go-is-domain v1.0.3 diff --git a/go.sum b/go.sum index e30d627c909..b3afe247eb1 100644 --- a/go.sum +++ b/go.sum @@ -223,8 +223,9 @@ github.com/ipfs/go-ipfs-exchange-offline v0.0.1 h1:P56jYKZF7lDDOLx5SotVh5KFxoY6C github.com/ipfs/go-ipfs-exchange-offline v0.0.1/go.mod h1:WhHSFCVYX36H/anEKQboAzpUws3x7UeEGkzQc3iNkM0= github.com/ipfs/go-ipfs-files v0.0.2/go.mod h1:INEFm0LL2LWXBhNJ2PMIIb2w45hpXgPjNoE7yA8Y1d4= github.com/ipfs/go-ipfs-files v0.0.3/go.mod h1:INEFm0LL2LWXBhNJ2PMIIb2w45hpXgPjNoE7yA8Y1d4= -github.com/ipfs/go-ipfs-files v0.0.4 h1:WzRCivcybUQch/Qh6v8LBRhKtRsjnwyiuOV09mK7mrE= github.com/ipfs/go-ipfs-files v0.0.4/go.mod h1:INEFm0LL2LWXBhNJ2PMIIb2w45hpXgPjNoE7yA8Y1d4= +github.com/ipfs/go-ipfs-files v0.0.6 h1:sMRtPiSmDrTA2FEiFTtk1vWgO2Dkg7bxXKJ+s8/cDAc= +github.com/ipfs/go-ipfs-files v0.0.6/go.mod h1:lVYE6sgAdtZN5825beJjSAHibw7WOBNPDWz5LaJeukg= github.com/ipfs/go-ipfs-flags v0.0.1/go.mod h1:RnXBb9WV53GSfTrSDVK61NLTFKvWc60n+K9EgCDh+rA= github.com/ipfs/go-ipfs-pinner v0.0.3 h1:ez/yNYYyH1W7DiCF/L29tmp6L7lBO8eqbJtPi2pHicA= github.com/ipfs/go-ipfs-pinner v0.0.3/go.mod h1:s4kFZWLWGDudN8Jyd/GTpt222A12C2snA2+OTdy/7p8= @@ -278,8 +279,8 @@ github.com/ipfs/go-todocounter v0.0.2 h1:9UBngSQhylg2UDcxSAtpkT+rEWFr26hDPXVStE8 github.com/ipfs/go-todocounter v0.0.2/go.mod h1:l5aErvQc8qKE2r7NDMjmq5UNAvuZy0rC8BHOplkWvZ4= github.com/ipfs/go-unixfs v0.0.4/go.mod h1:eIo/p9ADu/MFOuyxzwU+Th8D6xoxU//r590vUpWyfz8= github.com/ipfs/go-unixfs v0.1.0/go.mod h1:lysk5ELhOso8+Fed9U1QTGey2ocsfaZ18h0NCO2Fj9s= -github.com/ipfs/go-unixfs v0.2.1 h1:g51t9ODICFZ3F51FPivm8dE7NzYcdAQNUL9wGP5AYa0= -github.com/ipfs/go-unixfs v0.2.1/go.mod h1:IwAAgul1UQIcNZzKPYZWOCijryFBeCV79cNubPzol+k= +github.com/ipfs/go-unixfs v0.2.3 h1:VsZwK3Z6+rjFxha87GBrp3kZHDsztSIuKlsScr3Iw4s= +github.com/ipfs/go-unixfs v0.2.3/go.mod h1:SUdisfUjNoSDzzhGVxvCL9QO/nKdwXdr+gbMUdqcbYw= github.com/ipfs/go-verifcid v0.0.1 h1:m2HI7zIuR5TFyQ1b79Da5N9dnnCP1vcu2QqawmWlK2E= github.com/ipfs/go-verifcid v0.0.1/go.mod h1:5Hrva5KBeIog4A+UpqlaIU+DEstipcJYQQZc0g37pY0= github.com/ipfs/interface-go-ipfs-core v0.2.5 h1:/rspOe8RbIxwtssEXHB+X9JXhOBDCQt8x50d2kFPXL8= From 62451039ecca70a5d6d836e822200adbf2d0dda7 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 26 Sep 2019 12:25:42 -0700 Subject: [PATCH 2/9] fix(gateway): serve the index with serveFile --- core/corehttp/gateway_handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/corehttp/gateway_handler.go b/core/corehttp/gateway_handler.go index e8839087ab7..2024df390e3 100644 --- a/core/corehttp/gateway_handler.go +++ b/core/corehttp/gateway_handler.go @@ -289,7 +289,7 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request } // write to request - http.ServeContent(w, r, "index.html", modtime, f) + i.serveFile(w, r, "index.html", modtime, f) return case resolver.ErrNoLink: // no index.html; noop From e8a6c0c050e398a5b5f896770c7e79226eea6e4d Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 26 Sep 2019 12:34:09 -0700 Subject: [PATCH 3/9] fix(gateway): gracefully handle files with unknown sizes in directory listings --- core/corehttp/gateway_handler.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/corehttp/gateway_handler.go b/core/corehttp/gateway_handler.go index 2024df390e3..f627d72c498 100644 --- a/core/corehttp/gateway_handler.go +++ b/core/corehttp/gateway_handler.go @@ -306,14 +306,14 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request var dirListing []directoryItem dirit := dir.Entries() for dirit.Next() { - // See comment above where originalUrlPath is declared. - s, err := dirit.Node().Size() - if err != nil { - internalWebError(w, err) - return + size := "?" + if s, err := dirit.Node().Size(); err == nil { + // Size may not be defined/supported. Continue anyways. + size = humanize.Bytes(uint64(s)) } - di := directoryItem{humanize.Bytes(uint64(s)), dirit.Name(), gopath.Join(originalUrlPath, dirit.Name())} + // See comment above where originalUrlPath is declared. + di := directoryItem{size, dirit.Name(), gopath.Join(originalUrlPath, dirit.Name())} dirListing = append(dirListing, di) } if dirit.Err() != nil { From 3859f08bf79c9a68933c9d398f834c5c32462664 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 26 Sep 2019 13:26:51 -0700 Subject: [PATCH 4/9] fix(gateway): better seeking/sized 1. Require files to have known sizes. We can add support for unknown sizes _later_ but we can't use ServeContent for those files. 2. Replace the `sizeReadSeeker` with a `lazySeeker`. This one makes no assumptions about how it's used so we're less likely to run into weird bugs. --- core/corehttp/gateway_handler.go | 28 +++++---------- core/corehttp/lazyseek.go | 60 ++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 19 deletions(-) create mode 100644 core/corehttp/lazyseek.go diff --git a/core/corehttp/gateway_handler.go b/core/corehttp/gateway_handler.go index f627d72c498..38f1f53b350 100644 --- a/core/corehttp/gateway_handler.go +++ b/core/corehttp/gateway_handler.go @@ -372,29 +372,19 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request } } -type sizeReadSeeker interface { +type sized interface { Size() (int64, error) - - io.ReadSeeker -} - -type sizeSeeker struct { - sizeReadSeeker } -func (s *sizeSeeker) Seek(offset int64, whence int) (int64, error) { - if whence == io.SeekEnd && offset == 0 { - return s.Size() +func (i *gatewayHandler) serveFile(w http.ResponseWriter, req *http.Request, name string, modtime time.Time, file files.File) { + size, err := file.Size() + if err != nil { + http.Error(w, "cannot serve files with unknown sizes", http.StatusBadGateway) + return } - - return s.sizeReadSeeker.Seek(offset, whence) -} - -func (i *gatewayHandler) serveFile(w http.ResponseWriter, req *http.Request, name string, modtime time.Time, content io.ReadSeeker) { - if sp, ok := content.(sizeReadSeeker); ok { - content = &sizeSeeker{ - sizeReadSeeker: sp, - } + content := &lazySeeker{ + size: size, + reader: file, } ctype := mime.TypeByExtension(gopath.Ext(name)) diff --git a/core/corehttp/lazyseek.go b/core/corehttp/lazyseek.go new file mode 100644 index 00000000000..4c5624bcf9d --- /dev/null +++ b/core/corehttp/lazyseek.go @@ -0,0 +1,60 @@ +package corehttp + +import ( + "fmt" + "io" +) + +// The HTTP server uses seek to determine the file size. Actually _seeking_ can +// be slow so we wrap the seeker in a _lazy_ seeker. +type lazySeeker struct { + reader io.ReadSeeker + + size int64 + offset int64 + realOffset int64 +} + +func (s *lazySeeker) Seek(offset int64, whence int) (int64, error) { + switch whence { + case io.SeekEnd: + return s.Seek(s.size+offset, io.SeekStart) + case io.SeekCurrent: + return s.Seek(s.offset+offset, io.SeekStart) + case io.SeekStart: + if offset < 0 { + return s.offset, fmt.Errorf("invalid seek offset") + } + s.offset = offset + return s.offset, nil + default: + return s.offset, fmt.Errorf("invalid whence: %d", whence) + } +} + +func (s *lazySeeker) Read(b []byte) (int, error) { + // If we're past the end, EOF. + if s.offset >= s.size { + return 0, io.EOF + } + + // actually seek + for s.offset != s.realOffset { + off, err := s.reader.Seek(s.offset, io.SeekStart) + if err != nil { + return 9, err + } + s.realOffset = off + } + off, err := s.reader.Read(b) + s.realOffset += int64(off) + s.offset += int64(off) + return off, err +} + +func (s *lazySeeker) Close() error { + if closer, ok := s.reader.(io.Closer); ok { + return closer.Close() + } + return nil +} From 1a06fb6e2f721b5443495c42d066125ef98edb6c Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 26 Sep 2019 13:53:54 -0700 Subject: [PATCH 5/9] fix(gateway): correct symlink content type We should be _resolving_ symlinks (sometimes, still need to figure out when to do this WRT IPNS). However, that's a larger feature. --- core/corehttp/gateway_handler.go | 40 +++++++++++++++++++------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/core/corehttp/gateway_handler.go b/core/corehttp/gateway_handler.go index 38f1f53b350..55128842a64 100644 --- a/core/corehttp/gateway_handler.go +++ b/core/corehttp/gateway_handler.go @@ -382,28 +382,36 @@ func (i *gatewayHandler) serveFile(w http.ResponseWriter, req *http.Request, nam http.Error(w, "cannot serve files with unknown sizes", http.StatusBadGateway) return } + content := &lazySeeker{ size: size, reader: file, } - ctype := mime.TypeByExtension(gopath.Ext(name)) - if ctype == "" { - buf := make([]byte, 512) - n, _ := io.ReadFull(content, buf[:]) - ctype = http.DetectContentType(buf[:n]) - _, err := content.Seek(0, io.SeekStart) - if err != nil { - http.Error(w, "seeker can't seek", http.StatusInternalServerError) - return + var ctype string + if _, isSymlink := file.(*files.Symlink); isSymlink { + // We should be smarter about resolving symlinks but this is the + // "most correct" we can be without doing that. + ctype = "inode/symlink" + } else { + ctype = mime.TypeByExtension(gopath.Ext(name)) + if ctype == "" { + buf := make([]byte, 512) + n, _ := io.ReadFull(content, buf[:]) + ctype = http.DetectContentType(buf[:n]) + _, err := content.Seek(0, io.SeekStart) + if err != nil { + http.Error(w, "seeker can't seek", http.StatusInternalServerError) + return + } + } + // Strip the encoding from the HTML Content-Type header and let the + // browser figure it out. + // + // Fixes https://github.com/ipfs/go-ipfs/issues/2203 + if strings.HasPrefix(ctype, "text/html;") { + ctype = "text/html" } - } - // Strip the encoding from the HTML Content-Type header and let the - // browser figure it out. - // - // Fixes https://github.com/ipfs/go-ipfs/issues/2203 - if strings.HasPrefix(ctype, "text/html;") { - ctype = "text/html" } w.Header().Set("Content-Type", ctype) From 9a9ec021cf886e752eaf29cfccf8d6c08b39f0a5 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 26 Sep 2019 14:10:05 -0700 Subject: [PATCH 6/9] test(sharness): add gateway symlink test --- test/sharness/t0113-gateway-symlink.sh | 38 ++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100755 test/sharness/t0113-gateway-symlink.sh diff --git a/test/sharness/t0113-gateway-symlink.sh b/test/sharness/t0113-gateway-symlink.sh new file mode 100755 index 00000000000..d1f94bde7e9 --- /dev/null +++ b/test/sharness/t0113-gateway-symlink.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# +# Copyright (c) Protocol Labs + +test_description="Test symlink support on the HTTP gateway" + +. lib/test-lib.sh + +test_init_ipfs +test_launch_ipfs_daemon + + +test_expect_success "Create a test directory with symlinks" ' + mkdir testfiles && + echo "content" > testfiles/foo && + ln -s foo testfiles/bar && + test_cmp testfiles/foo testfiles/bar +' + +test_expect_success "Add the test directory" ' + HASH=$(ipfs add -Qr testfiles) +' + +test_expect_success "Test the directory listing" ' + curl "$GWAY_ADDR/ipfs/$HASH" > list_response && + test_should_contain ">foo<" list_response && + test_should_contain ">bar<" list_response +' + +test_expect_success "Test the symlink" ' + curl "$GWAY_ADDR/ipfs/$HASH/bar" > bar_actual && + echo -n "foo" > bar_expected && + test_cmp bar_expected bar_actual +' + +test_kill_ipfs_daemon + +test_done From 453b78962b790eeac117172d81385a5189716aae Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 26 Sep 2019 14:19:46 -0700 Subject: [PATCH 7/9] chore(gateway): remove dead code --- core/corehttp/gateway_handler.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/corehttp/gateway_handler.go b/core/corehttp/gateway_handler.go index 55128842a64..a501e21ddde 100644 --- a/core/corehttp/gateway_handler.go +++ b/core/corehttp/gateway_handler.go @@ -372,10 +372,6 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request } } -type sized interface { - Size() (int64, error) -} - func (i *gatewayHandler) serveFile(w http.ResponseWriter, req *http.Request, name string, modtime time.Time, file files.File) { size, err := file.Size() if err != nil { From 6cb03d4dfd87770fcce330cbfe5e36126c63339a Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 3 Jan 2020 18:07:20 -0800 Subject: [PATCH 8/9] fix(gateway): fix seek read length typo --- core/corehttp/lazyseek.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/corehttp/lazyseek.go b/core/corehttp/lazyseek.go index 4c5624bcf9d..2a379dc918a 100644 --- a/core/corehttp/lazyseek.go +++ b/core/corehttp/lazyseek.go @@ -42,7 +42,7 @@ func (s *lazySeeker) Read(b []byte) (int, error) { for s.offset != s.realOffset { off, err := s.reader.Seek(s.offset, io.SeekStart) if err != nil { - return 9, err + return 0, err } s.realOffset = off } From c64eb11992f755f88424afa2d8dacfc8b16d09fe Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 3 Jan 2020 18:08:28 -0800 Subject: [PATCH 9/9] test(gateway): test the lazy seeker --- core/corehttp/lazyseek_test.go | 137 +++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 core/corehttp/lazyseek_test.go diff --git a/core/corehttp/lazyseek_test.go b/core/corehttp/lazyseek_test.go new file mode 100644 index 00000000000..144e57d0f27 --- /dev/null +++ b/core/corehttp/lazyseek_test.go @@ -0,0 +1,137 @@ +package corehttp + +import ( + "fmt" + "io" + "io/ioutil" + "strings" + "testing" +) + +type badSeeker struct { + io.ReadSeeker +} + +var badSeekErr = fmt.Errorf("I'm a bad seeker") + +func (bs badSeeker) Seek(offset int64, whence int) (int64, error) { + off, err := bs.ReadSeeker.Seek(0, io.SeekCurrent) + if err != nil { + panic(err) + } + return off, badSeekErr +} + +func TestLazySeekerError(t *testing.T) { + underlyingBuffer := strings.NewReader("fubar") + s := &lazySeeker{ + reader: badSeeker{underlyingBuffer}, + size: underlyingBuffer.Size(), + } + off, err := s.Seek(0, io.SeekEnd) + if err != nil { + t.Fatal(err) + } + if off != s.size { + t.Fatal("expected to seek to the end") + } + + // shouldn't have actually seeked. + b, err := ioutil.ReadAll(s) + if err != nil { + t.Fatal(err) + } + if len(b) != 0 { + t.Fatal("expected to read nothing") + } + + // shouldn't need to actually seek. + off, err = s.Seek(0, io.SeekStart) + if err != nil { + t.Fatal(err) + } + if off != 0 { + t.Fatal("expected to seek to the start") + } + b, err = ioutil.ReadAll(s) + if err != nil { + t.Fatal(err) + } + if string(b) != "fubar" { + t.Fatal("expected to read string") + } + + // should fail the second time. + off, err = s.Seek(0, io.SeekStart) + if err != nil { + t.Fatal(err) + } + if off != 0 { + t.Fatal("expected to seek to the start") + } + // right here... + b, err = ioutil.ReadAll(s) + if err == nil { + t.Fatalf("expected an error, got output %s", string(b)) + } + if err != badSeekErr { + t.Fatalf("expected a bad seek error, got %s", err) + } + if len(b) != 0 { + t.Fatalf("expected to read nothing") + } +} + +func TestLazySeeker(t *testing.T) { + underlyingBuffer := strings.NewReader("fubar") + s := &lazySeeker{ + reader: underlyingBuffer, + size: underlyingBuffer.Size(), + } + expectByte := func(b byte) { + t.Helper() + var buf [1]byte + n, err := io.ReadFull(s, buf[:]) + if err != nil { + t.Fatal(err) + } + if n != 1 { + t.Fatalf("expected to read one byte, read %d", n) + } + if buf[0] != b { + t.Fatalf("expected %b, got %b", b, buf[0]) + } + } + expectSeek := func(whence int, off, expOff int64, expErr string) { + t.Helper() + n, err := s.Seek(off, whence) + if expErr == "" { + if err != nil { + t.Fatal("unexpected seek error: ", err) + } + } else { + if err == nil || err.Error() != expErr { + t.Fatalf("expected %s, got %s", err, expErr) + } + } + if n != expOff { + t.Fatalf("expected offset %d, got, %d", expOff, n) + } + } + + expectSeek(io.SeekEnd, 0, s.size, "") + b, err := ioutil.ReadAll(s) + if err != nil { + t.Fatal(err) + } + if len(b) != 0 { + t.Fatal("expected to read nothing") + } + expectSeek(io.SeekEnd, -1, s.size-1, "") + expectByte('r') + expectSeek(io.SeekStart, 0, 0, "") + expectByte('f') + expectSeek(io.SeekCurrent, 1, 2, "") + expectByte('b') + expectSeek(io.SeekCurrent, -100, 3, "invalid seek offset") +}