Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BOUNTY] Directory page UI improvements #7536

Merged
merged 5 commits into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions assets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ commit.
```bash
> go generate .
> git add bindata.go
> git add bindata_version_hash.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lidel it looks like you forgot to run this while rebasing as I don't see any changes to the bindata_version_hash.go file.

Copy link
Contributor

@aschmahmann aschmahmann Aug 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Stebalien merged #7609 and I rebased on top and added the relevant files. @lidel if you could double check in the morning that things look right that'd be great.

Copy link
Member

@lidel lidel Aug 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aschmahmann retested locally and looks good, super happy about deterministic assets 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

absolutely, it also helped with the fact that different OSes would give different modes.

> go mod tidy
> git commit --amend --no-edit

Expand Down
4 changes: 2 additions & 2 deletions assets/bindata.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion assets/bindata_version_hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
package assets

const (
BindataVersionHash = "60ae1cfa3f31134017737125db89f905dd3eea98"
BindataVersionHash = "514e5ae28d8adb84955801b56ef47aca44bf9cc8"
)
42 changes: 37 additions & 5 deletions core/corehttp/gateway_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,20 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
size = humanize.Bytes(uint64(s))
}

hash := ""
if r, err := i.api.ResolvePath(r.Context(), ipath.Join(resolvedPath, dirit.Name())); err == nil {
// Path may not be resolved. Continue anyways.
hash = r.Cid().String()
}

// See comment above where originalUrlPath is declared.
di := directoryItem{size, dirit.Name(), gopath.Join(originalUrlPath, dirit.Name())}
di := directoryItem{
Size: size,
Name: dirit.Name(),
Path: gopath.Join(originalUrlPath, dirit.Name()),
Hash: hash,
ShortHash: shortHash(hash),
}
dirListing = append(dirListing, di)
}
if dirit.Err() != nil {
Expand Down Expand Up @@ -359,14 +371,34 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
}
}

size := "?"
if s, err := dir.Size(); err == nil {
// Size may not be defined/supported. Continue anyways.
size = humanize.Bytes(uint64(s))
}

hash := resolvedPath.Cid().String()

// Storage for gateway URL to be used when linking to other rootIDs. This
// will be blank unless subdomain resolution is being used for this request.
var gwURL string

// Get gateway hostname and build gateway URL.
if h, ok := r.Context().Value("gw-hostname").(string); ok {
gwURL = "//" + h
} else {
gwURL = ""
}

// See comment above where originalUrlPath is declared.
tplData := listingTemplateData{
Listing: dirListing,
Path: urlPath,
BackLink: backLink,
Hash: hash,
GatewayURL: gwURL,
Listing: dirListing,
Size: size,
Path: urlPath,
Breadcrumbs: breadcrumbs(urlPath),
BackLink: backLink,
Hash: hash,
}

err = listingTemplate.Execute(w, tplData)
Expand Down
49 changes: 44 additions & 5 deletions core/corehttp/gateway_indexPage.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,61 @@ import (
"strings"

"github.com/ipfs/go-ipfs/assets"
ipfspath "github.com/ipfs/go-path"
)

// structs for directory listing
type listingTemplateData struct {
Listing []directoryItem
Path string
BackLink string
Hash string
GatewayURL string
Listing []directoryItem
Size string
Path string
Breadcrumbs []breadcrumb
BackLink string
Hash string
}

type directoryItem struct {
Size string
Size string
Name string
Path string
Hash string
ShortHash string
}

type breadcrumb struct {
Name string
Path string
}

func breadcrumbs(urlPath string) []breadcrumb {
var ret []breadcrumb

p, err := ipfspath.ParsePath(urlPath)
if err != nil {
// No breadcrumbs, fallback to bare Path in template
return ret
}

segs := p.Segments()
for i, seg := range segs {
if i == 0 {
ret = append(ret, breadcrumb{Name: seg})
} else {
ret = append(ret, breadcrumb{
Name: seg,
Path: "/" + strings.Join(segs[0:i+1], "/"),
})
}
}

return ret
}

func shortHash(hash string) string {
return (hash[0:4] + "\u2026" + hash[len(hash)-4:])
}

var listingTemplate *template.Template

func init() {
Expand Down
16 changes: 11 additions & 5 deletions core/corehttp/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io/ioutil"
"net/http"
"net/http/httptest"
"regexp"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -154,6 +155,11 @@ func newTestServerAndNode(t *testing.T, ns mockNamesys) (*httptest.Server, iface
return ts, api, n.Context()
}

func matchPathOrBreadcrumbs(s string, expected string) bool {
matched, _ := regexp.MatchString("Index of\n[\t ]*"+regexp.QuoteMeta(expected), s)
return matched
}

func TestGatewayGet(t *testing.T) {
ns := mockNamesys{}
ts, api, ctx := newTestServerAndNode(t, ns)
Expand Down Expand Up @@ -442,7 +448,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
s := string(body)
t.Logf("body: %s\n", string(body))

if !strings.Contains(s, "Index of /ipns/example.net/foo? #<'/") {
if !matchPathOrBreadcrumbs(s, "/ipns/<a href=\"/ipns/example.net\">example.net</a>/<a href=\"/ipns/example.net/foo%3F%20%23%3C%27\">foo? #&lt;&#39;</a>") {
t.Fatalf("expected a path in directory listing")
}
if !strings.Contains(s, "<a href=\"/foo%3F%20%23%3C%27/./..\">") {
Expand Down Expand Up @@ -475,7 +481,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
s = string(body)
t.Logf("body: %s\n", string(body))

if !strings.Contains(s, "Index of /") {
if !matchPathOrBreadcrumbs(s, "/") {
t.Fatalf("expected a path in directory listing")
}
if !strings.Contains(s, "<a href=\"/\">") {
Expand Down Expand Up @@ -508,7 +514,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
s = string(body)
t.Logf("body: %s\n", string(body))

if !strings.Contains(s, "Index of /ipns/example.net/foo? #&lt;&#39;/bar/") {
if !matchPathOrBreadcrumbs(s, "/ipns/<a href=\"/ipns/example.net\">example.net</a>/<a href=\"/ipns/example.net/foo%3F%20%23%3C%27\">foo? #&lt;&#39;</a>/<a href=\"/ipns/example.net/foo%3F%20%23%3C%27/bar\">bar</a>") {
t.Fatalf("expected a path in directory listing")
}
if !strings.Contains(s, "<a href=\"/foo%3F%20%23%3C%27/bar/./..\">") {
Expand Down Expand Up @@ -542,7 +548,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
s = string(body)
t.Logf("body: %s\n", string(body))

if !strings.Contains(s, "Index of /ipns/example.net") {
if !matchPathOrBreadcrumbs(s, "/ipns/<a href=\"/ipns/example.net\">example.net</a>") {
t.Fatalf("expected a path in directory listing")
}
if !strings.Contains(s, "<a href=\"/good-prefix/\">") {
Expand Down Expand Up @@ -584,7 +590,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
s = string(body)
t.Logf("body: %s\n", string(body))

if !strings.Contains(s, "Index of /") {
if !matchPathOrBreadcrumbs(s, "/") {
t.Fatalf("expected a path in directory listing")
}
if !strings.Contains(s, "<a href=\"/\">") {
Expand Down
6 changes: 5 additions & 1 deletion core/corehttp/hostname.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ func HostnameOption() ServeOption {
if gw, hostname, ns, rootID, ok := knownSubdomainDetails(host, knownGateways); ok {
// Looks like we're using a known gateway in subdomain mode.

// Add gateway hostname context for linking to other root ids.
// Example: localhost/ipfs/{cid}
ctx := context.WithValue(r.Context(), "gw-hostname", hostname)

// Assemble original path prefix.
pathPrefix := "/" + ns + "/" + rootID

Expand Down Expand Up @@ -197,7 +201,7 @@ func HostnameOption() ServeOption {
r.URL.Path = pathPrefix + r.URL.Path

// Serve path request
childMux.ServeHTTP(w, r)
childMux.ServeHTTP(w, r.WithContext(ctx))
return
}
// We don't have a known gateway. Fallback on DNSLink lookup
Expand Down
2 changes: 1 addition & 1 deletion test/sharness/t0111-gateway-writeable.sh
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ test_expect_success "HTTP GET empty directory" '
URL="http://127.0.0.1:$port/ipfs/$HASH_EMPTY_DIR/" &&
echo "GET $URL" &&
curl -so outfile "$URL" 2>curl_getEmpty.out &&
grep "Index of /ipfs/$HASH_EMPTY_DIR/" outfile
cat outfile | tr -s "\n" " " | grep "Index of /ipfs/<a href=\"/ipfs/$HASH_EMPTY_DIR\">$HASH_EMPTY_DIR</a>"
'

test_expect_success "HTTP PUT file to construct a hierarchy" '
Expand Down
36 changes: 22 additions & 14 deletions test/sharness/t0114-gateway-subdomains.sh
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,11 @@ test_expect_success "Add test text file" '
echo CIDv0to1=${CIDv0to1}
'

# Directory tree crafted to test for edge cases like "/ipfs/ipfs/ipns/bar"
test_expect_success "Add the test directory" '
mkdir -p testdirlisting/subdir1/subdir2 &&
mkdir -p testdirlisting/ipfs/ipns &&
echo "hello" > testdirlisting/hello &&
echo "subdir2-bar" > testdirlisting/subdir1/subdir2/bar &&
echo "text-file-content" > testdirlisting/ipfs/ipns/bar &&
mkdir -p testdirlisting/api &&
mkdir -p testdirlisting/ipfs &&
echo "I am a txt file" > testdirlisting/api/file.txt &&
Expand Down Expand Up @@ -269,18 +270,18 @@ DIR_HOSTNAME="${DIR_CID}.ipfs.localhost:$GWAY_PORT"
test_expect_success "valid file and subdirectory paths in directory listing at {cid}.ipfs.localhost" '
curl -s --resolve $DIR_HOSTNAME:127.0.0.1 "http://$DIR_HOSTNAME" > list_response &&
test_should_contain "<a href=\"/hello\">hello</a>" list_response &&
test_should_contain "<a href=\"/subdir1\">subdir1</a>" list_response
test_should_contain "<a href=\"/ipfs\">ipfs</a>" list_response
'

test_expect_success "valid parent directory path in directory listing at {cid}.ipfs.localhost/sub/dir" '
curl -s --resolve $DIR_HOSTNAME:127.0.0.1 "http://$DIR_HOSTNAME/subdir1/subdir2/" > list_response &&
test_should_contain "<a href=\"/subdir1/subdir2/./..\">..</a>" list_response &&
test_should_contain "<a href=\"/subdir1/subdir2/bar\">bar</a>" list_response
curl -s --resolve $DIR_HOSTNAME:127.0.0.1 "http://$DIR_HOSTNAME/ipfs/ipns/" > list_response &&
test_should_contain "<a href=\"/ipfs/ipns/./..\">..</a>" list_response &&
test_should_contain "<a href=\"/ipfs/ipns/bar\">bar</a>" list_response
'

test_expect_success "request for deep path resource at {cid}.ipfs.localhost/sub/dir/file" '
curl -s --resolve $DIR_HOSTNAME:127.0.0.1 "http://$DIR_HOSTNAME/subdir1/subdir2/bar" > list_response &&
test_should_contain "subdir2-bar" list_response
curl -s --resolve $DIR_HOSTNAME:127.0.0.1 "http://$DIR_HOSTNAME/ipfs/ipns/bar" > list_response &&
test_should_contain "text-file-content" list_response
'


Expand Down Expand Up @@ -422,18 +423,25 @@ DIR_FQDN="${DIR_CID}.ipfs.example.com"
test_expect_success "valid file and directory paths in directory listing at {cid}.ipfs.example.com" '
curl -s -H "Host: $DIR_FQDN" http://127.0.0.1:$GWAY_PORT > list_response &&
test_should_contain "<a href=\"/hello\">hello</a>" list_response &&
test_should_contain "<a href=\"/subdir1\">subdir1</a>" list_response
test_should_contain "<a href=\"/ipfs\">ipfs</a>" list_response
'

test_expect_success "valid parent directory path in directory listing at {cid}.ipfs.example.com/sub/dir" '
curl -s -H "Host: $DIR_FQDN" http://127.0.0.1:$GWAY_PORT/subdir1/subdir2/ > list_response &&
test_should_contain "<a href=\"/subdir1/subdir2/./..\">..</a>" list_response &&
test_should_contain "<a href=\"/subdir1/subdir2/bar\">bar</a>" list_response
curl -s -H "Host: $DIR_FQDN" http://127.0.0.1:$GWAY_PORT/ipfs/ipns/ > list_response &&
test_should_contain "<a href=\"/ipfs/ipns/./..\">..</a>" list_response &&
test_should_contain "<a href=\"/ipfs/ipns/bar\">bar</a>" list_response
'

# Note we test for sneaky subdir names {cid}.ipfs.example.com/ipfs/ipns/ :^)
test_expect_success "valid breadcrumb links in the header of directory listing at {cid}.ipfs.example.com/sub/dir" '
curl -s -H "Host: $DIR_FQDN" http://127.0.0.1:$GWAY_PORT/ipfs/ipns/ > list_response &&
test_should_contain "Index of" list_response &&
test_should_contain "/ipfs/<a href=\"//example.com/ipfs/${DIR_CID}\">${DIR_CID}</a>/<a href=\"//example.com/ipfs/${DIR_CID}/ipfs\">ipfs</a>/<a href=\"//example.com/ipfs/${DIR_CID}/ipfs/ipns\">ipns</a>" list_response
'

test_expect_success "request for deep path resource {cid}.ipfs.example.com/sub/dir/file" '
curl -s -H "Host: $DIR_FQDN" http://127.0.0.1:$GWAY_PORT/subdir1/subdir2/bar > list_response &&
test_should_contain "subdir2-bar" list_response
curl -s -H "Host: $DIR_FQDN" http://127.0.0.1:$GWAY_PORT/ipfs/ipns/bar > list_response &&
test_should_contain "text-file-content" list_response
'

# *.ipns.example.com
Expand Down