From 6fe189a4cd375f083ae930c300b1b9269aa7b771 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 6 Jul 2011 18:38:27 -0700 Subject: [PATCH] publishing: new URLs, tests. Yay photo galleries. Change-Id: I9081b49faf95f5193945032cbc6711f412bca198 --- lib/go/camli/search/handler.go | 18 +++- server/go/camlistored/publish.go | 123 ++++++++++++++------------ server/go/camlistored/publish_test.go | 89 ++++++++++++++----- 3 files changed, 147 insertions(+), 83 deletions(-) diff --git a/lib/go/camli/search/handler.go b/lib/go/camli/search/handler.go index a70491e69..6d9d58915 100644 --- a/lib/go/camli/search/handler.go +++ b/lib/go/camli/search/handler.go @@ -369,6 +369,9 @@ func (b *DescribedBlob) PeerBlob(br *blobref.BlobRef) *DescribedBlob { // somewhat redundant "Secure" in the name) and should be paranoid // against e.g. random user/attacker-control attributes making links // to other blobs. +// +// TODO: don't linear scan here. rewrite this in terms of ResolvePrefixHop, +// passing down some policy perhaps? or maybe that's enough. func (b *DescribedBlob) HasSecureLinkTo(other *blobref.BlobRef) bool { if b == nil || other == nil { return false @@ -437,7 +440,9 @@ func (sh *Handler) NewDescribeRequest() *DescribeRequest { } } -func (sh *Handler) ResolveMemberPrefix(parent *blobref.BlobRef, prefix string) (child *blobref.BlobRef, err os.Error) { +// Given a blobref and a few hex characters of the digest of the next hop, return the complete +// blobref of the prefix, if that's a valid next hop. +func (sh *Handler) ResolvePrefixHop(parent *blobref.BlobRef, prefix string) (child *blobref.BlobRef, err os.Error) { // TODO: this is a linear scan right now. this should be // optimized to use a new database table of members so this is // a quick lookup. in the meantime it should be in memcached @@ -455,9 +460,14 @@ func (sh *Handler) ResolveMemberPrefix(parent *blobref.BlobRef, prefix string) ( if !ok { return nil, fmt.Errorf("Failed to describe member %q in parent %q", prefix, parent) } - for _, member := range des.Members() { - if strings.HasPrefix(member.BlobRef.Digest(), prefix) { - return member.BlobRef, nil + if des.Permanode != nil { + if cr, ok := des.ContentRef(); ok && strings.HasPrefix(cr.Digest(), prefix) { + return cr, nil + } + for _, member := range des.Members() { + if strings.HasPrefix(member.BlobRef.Digest(), prefix) { + return member.BlobRef, nil + } } } return nil, fmt.Errorf("Member prefix %q not found in %q", prefix, parent) diff --git a/server/go/camlistored/publish.go b/server/go/camlistored/publish.go index 39273ef78..43fb75d52 100644 --- a/server/go/camlistored/publish.go +++ b/server/go/camlistored/publish.go @@ -148,6 +148,8 @@ type publishRequest struct { base, suffix, subres string rootpn *blobref.BlobRef subject *blobref.BlobRef + inSubjectChain map[string]bool // blobref -> true + subjectBasePath string // A describe request that we can reuse, sharing its map of // blobs already described. @@ -165,14 +167,16 @@ func (ph *PublishHandler) NewRequest(rw http.ResponseWriter, req *http.Request) } rootpn, _ := ph.rootPermanode() return &publishRequest{ - ph: ph, - rw: rw, - req: req, - suffix: suffix, - base: req.Header.Get("X-PrefixHandler-PathBase"), - subres: res, - rootpn: rootpn, - dr: ph.Search.NewDescribeRequest(), + ph: ph, + rw: rw, + req: req, + suffix: suffix, + base: req.Header.Get("X-PrefixHandler-PathBase"), + subres: res, + rootpn: rootpn, + dr: ph.Search.NewDescribeRequest(), + inSubjectChain: make(map[string]bool), + subjectBasePath: "", } } @@ -181,8 +185,8 @@ func (pr *publishRequest) Debug() bool { } func (pr *publishRequest) SubresourceType() string { - if parts := strings.SplitN(pr.subres, "/", 2); len(parts) > 0 { - return parts[0] + if len(pr.subres) >= 3 && strings.HasPrefix(pr.subres, "/=") { + return pr.subres[2:3] } return "" } @@ -193,14 +197,21 @@ func (pr *publishRequest) SubresFileURL(path []*blobref.BlobRef, fileName string func (pr *publishRequest) SubresThumbnailURL(path []*blobref.BlobRef, fileName string, maxDimen int) string { var buf bytes.Buffer - resType := "img" + resType := "i" if maxDimen == -1 { - resType = "file" + resType = "f" + } + fmt.Fprintf(&buf, "%s", pr.subjectBasePath) + if !strings.Contains(pr.subjectBasePath, "/-/") { + buf.Write([]byte("/-")) } - fmt.Fprintf(&buf, "%s%s/-/%s", pr.base, pr.suffix, resType) for _, br := range path { - fmt.Fprintf(&buf, "/%s", br) + if pr.inSubjectChain[br.String()] { + continue + } + fmt.Fprintf(&buf, "/h%s", br.DigestPrefix(10)) } + fmt.Fprintf(&buf, "/=%s", resType) fmt.Fprintf(&buf, "/%s", http.URLEscape(fileName)) if maxDimen != -1 { fmt.Fprintf(&buf, "?mw=%d&mh=%d", maxDimen, maxDimen) @@ -208,15 +219,22 @@ func (pr *publishRequest) SubresThumbnailURL(path []*blobref.BlobRef, fileName s return buf.String() } -var memberRE = regexp.MustCompile(`^/?m([0-9a-f]+)`) +var memberRE = regexp.MustCompile(`^/?h([0-9a-f]+)`) func (pr *publishRequest) findSubject() os.Error { + if strings.HasPrefix(pr.suffix, "=s/") { + pr.subres = "/" + pr.suffix + return nil + } + subject, err := pr.ph.lookupPathTarget(pr.rootpn, pr.suffix) if err != nil { return err } + pr.inSubjectChain[subject.String()] = true + pr.subjectBasePath = pr.base + pr.suffix - // Chase /m members in suffix. + // Chase /h hops in suffix. for { m := memberRE.FindStringSubmatch(pr.subres) if m == nil { @@ -228,11 +246,14 @@ func (pr *publishRequest) findSubject() os.Error { return fmt.Errorf("Error looking up potential member %q in describe of subject %q: %v", memberPrefix, subject, err) } - subject, err = pr.ph.Search.ResolveMemberPrefix(subject, memberPrefix) + + subject, err = pr.ph.Search.ResolvePrefixHop(subject, memberPrefix) if err != nil { return err } + pr.inSubjectChain[subject.String()] = true pr.subres = pr.subres[len(match):] + pr.subjectBasePath = addPathComponent(pr.subjectBasePath, match) } pr.subject = subject @@ -268,14 +289,14 @@ func (pr *publishRequest) serveHTTP() { switch pr.SubresourceType() { case "": pr.serveSubject() - case "blob": + case "b": // TODO: download a raw blob - case "file": + case "f": // file download pr.serveSubresFileDownload() - case "img": + case "i": // image, scaled pr.serveSubresImage() - case "static": - pr.req.URL.Path = pr.subres[len("static"):] + case "s": // static + pr.req.URL.Path = pr.subres[len("/=s"):] pr.ph.staticHandler.ServeHTTP(pr.rw, pr.req) default: pr.rw.WriteHeader(400) @@ -288,14 +309,21 @@ func (pr *publishRequest) pf(format string, args ...interface{}) { } func (pr *publishRequest) staticPath(fileName string) string { - return pr.base + "-/static/" + fileName + return pr.base + "=s/" + fileName +} + +func addPathComponent(base, addition string) string { + if !strings.HasPrefix(addition, "/") { + addition = "/" + addition + } + if strings.Contains(base, "/-/") { + return base + addition + } + return base + "/-" + addition } func (pr *publishRequest) memberPath(member *blobref.BlobRef) string { - if strings.Contains(pr.req.URL.Path, "/-/") { - return pr.req.URL.Path + "/m" + member.DigestPrefix(10) - } - return pr.req.URL.Path + "/-/m" + member.DigestPrefix(10) + return addPathComponent(pr.subjectBasePath, "/h"+member.DigestPrefix(10)) } func (pr *publishRequest) serveSubject() { @@ -407,45 +435,21 @@ func (pr *publishRequest) serveSubresImage() { params := pr.req.URL.Query() mw, _ := strconv.Atoi(params.Get("mw")) mh, _ := strconv.Atoi(params.Get("mh")) - if des, ok := pr.describeSubresAndValidatePath(); ok { - pr.serveScaledImage(des, mw, mh) + des, err := pr.dr.DescribeSync(pr.subject) + if err != nil { + log.Printf("error describing subject %q: %v", pr.subject, err) + return } + pr.serveScaledImage(des, mw, mh) } func (pr *publishRequest) serveSubresFileDownload() { - if des, ok := pr.describeSubresAndValidatePath(); ok { - pr.serveFileDownload(des) - } -} - -func (pr *publishRequest) describeSubresAndValidatePath() (des *search.DescribedBlob, ok bool) { - path := []*blobref.BlobRef{} - parts := strings.Split(pr.subres, "/") - if len(parts) < 3 { - http.Error(pr.rw, "expected at least 3 parts", 400) - return - } - for _, bstr := range parts[1 : len(parts)-1] { - if br := blobref.Parse(bstr); br != nil { - path = append(path, br) - } else { - http.Error(pr.rw, "bogus blobref in chain", 400) - return - } - } - - if !pr.validPathChain(path) { - http.Error(pr.rw, "not found or invalid path", 404) - return - } - - file := path[len(path)-1] - fileDes, err := pr.dr.DescribeSync(file) + des, err := pr.dr.DescribeSync(pr.subject) if err != nil { - http.Error(pr.rw, "describe error", 500) + log.Printf("error describing subject %q: %v", pr.subject, err) return } - return fileDes, true + pr.serveFileDownload(des) } func (pr *publishRequest) serveScaledImage(des *search.DescribedBlob, maxWidth, maxHeight int) { @@ -465,6 +469,7 @@ func (pr *publishRequest) serveScaledImage(des *search.DescribedBlob, maxWidth, func (pr *publishRequest) serveFileDownload(des *search.DescribedBlob) { fileref, fileinfo, ok := pr.fileSchemaRefFromBlob(des) if !ok { + log.Printf("Didn't get file schema from described blob %q", des.BlobRef) return } mime := "" diff --git a/server/go/camlistored/publish_test.go b/server/go/camlistored/publish_test.go index 6db02e2a8..cdb19c54d 100644 --- a/server/go/camlistored/publish_test.go +++ b/server/go/camlistored/publish_test.go @@ -24,28 +24,66 @@ import ( "camli/blobref" "camli/search" + "camli/httputil" "camli/test" ) type publishURLTest struct { - path string // input - - subject string // expected + path string // input + subject, subres string // expected } var publishURLTests = []publishURLTest{ + // URL to a single picture permanoe (returning its HTML wrapper page) { path: "/pics/singlepic", subject: "picpn-123", }, + + // URL to a gallery permanode (returning its HTML wrapper page) { path: "/pics/camping", subject: "gal-123", }, + + // URL to a picture permanode within a gallery (following one hop, returning HTML) { - path: "/pics/camping/-/m9876543210", + path: "/pics/camping/-/h9876543210", subject: "picpn-98765432100", }, + + // URL to a gallery -> picture permanode -> its file + // (following two hops, returning HTML) + { + path: "/pics/camping/-/h9876543210/hf00f00f00a", + subject: "picfile-f00f00f00a5", + }, + + // URL to a gallery -> picture permanode -> its file + // (following two hops, returning the file download) + { + path: "/pics/camping/-/h9876543210/hf00f00f00a/=f/marshmallow.jpg", + subject: "picfile-f00f00f00a5", + subres: "/=f/marshmallow.jpg", + }, + + // URL to a gallery -> picture permanode -> its file + // (following two hops, returning the file, scaled as an image) + { + path: "/pics/camping/-/h9876543210/hf00f00f00a/=i/marshmallow.jpg?mw=200&mh=200", + subject: "picfile-f00f00f00a5", + subres: "/=i/marshmallow.jpg", + }, + + // Path to a static file in the root. + // TODO: ditch these and use content-addressable javascript + css, having + // the server digest them on start, or rather part of fileembed. This is + // a short-term hack to unblock Lindsey. + { + path: "/pics/=s/camli.js", + subject: "", + subres: "/=s/camli.js", + }, } func TestPublishURLs(t *testing.T) { @@ -55,8 +93,8 @@ func TestPublishURLs(t *testing.T) { rootRef := blobref.MustParse("root-abc") camp0 := blobref.MustParse("picpn-98765432100") camp1 := blobref.MustParse("picpn-98765432111") - camp0f := blobref.MustParse("picfile-98765432f00") - camp1f := blobref.MustParse("picfile-98765432f10") + camp0f := blobref.MustParse("picfile-f00f00f00a5") + camp1f := blobref.MustParse("picfile-f00f00f00b6") rootName := "foo" @@ -68,13 +106,6 @@ func TestPublishURLs(t *testing.T) { RootName: rootName, Search: sh, } - rw := httptest.NewRecorder() - if !strings.HasPrefix(tt.path, "/pics/") { - panic("expected /pics/ prefix on " + tt.path) - } - req, _ := http.NewRequest("GET", "http://foo.com"+tt.path, nil) - req.Header.Set("X-PrefixHandler-PathBase", "/pics/") - req.Header.Set("X-PrefixHandler-PathSuffix", tt.path[len("/pics/"):]) idx.AddMeta(owner, "text/x-openpgp-public-key", 100) for _, br := range []*blobref.BlobRef{picNode, galRef, rootRef, camp0, camp1} { @@ -90,15 +121,33 @@ func TestPublishURLs(t *testing.T) { idx.AddClaim(owner, galRef, "add-attribute", "camliMember", camp1.String()) idx.AddClaim(owner, camp0, "set-attribute", "camliContent", camp0f.String()) idx.AddClaim(owner, camp1, "set-attribute", "camliContent", camp1f.String()) - pr := ph.NewRequest(rw, req) - err := pr.findSubject() - if err != nil { - t.Errorf("test #%d, findSubject: %v", ti, err) - continue + rw := httptest.NewRecorder() + if !strings.HasPrefix(tt.path, "/pics/") { + panic("expected /pics/ prefix on " + tt.path) } - if pr.subject.String() != tt.subject { - t.Errorf("test #%d, got subject %q, want %q", ti, pr.subject, tt.subject) + req, _ := http.NewRequest("GET", "http://foo.com"+tt.path, nil) + + pfxh := &httputil.PrefixHandler{ + Prefix: "/pics/", + Handler: http.HandlerFunc(func(_ http.ResponseWriter, req *http.Request) { + pr := ph.NewRequest(rw, req) + + err := pr.findSubject() + if tt.subject != "" { + if err != nil { + t.Errorf("test #%d, findSubject: %v", ti, err) + return + } + if pr.subject.String() != tt.subject { + t.Errorf("test #%d, got subject %q, want %q", ti, pr.subject, tt.subject) + } + } + if pr.subres != tt.subres { + t.Errorf("test #%d, got subres %q, want %q", ti, pr.subres, tt.subres) + } + }), } + pfxh.ServeHTTP(rw, req) } }