Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

api,client,http,main,prod,storage: Add query param for publisher and solve flooding in recovery #2202

Merged
merged 19 commits into from
Jun 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
540187d
api/client,api:http,cmd/swarm,prod,storage: added flag for publisher …
santicomp2014 Jun 11, 2020
0463f7a
storage: changed recovery only when publisher is set
santicomp2014 Jun 11, 2020
0e8b18e
prod,netstore: added logs for flood test
santicomp2014 Jun 11, 2020
ca488fd
storage: remove publisher after recovery
santicomp2014 Jun 11, 2020
f5236ec
storage: removed test publisher removal
santicomp2014 Jun 11, 2020
754739b
prod: added log for publisher in getPinners
santicomp2014 Jun 11, 2020
0787628
api,api/client,apit/http,cmd/swarm,cmd/swarm-smoke: removed publisher…
santicomp2014 Jun 11, 2020
f1b0fbf
cmd/swarm: remove unused log in download path
santicomp2014 Jun 12, 2020
e25597e
api/client,api/http,cmd/swarm: added publisher to bzz-list for recove…
santicomp2014 Jun 12, 2020
6a9a30a
api/client,api/http,cmd/swarm,prod,storage: removed unused comments a…
santicomp2014 Jun 12, 2020
d5d3de9
Merge branch 'global-pinning' into global-pinning-fix-request-flooding
santicomp2014 Jun 12, 2020
19edb39
storage: changed comments in netstore for clarity in global pinning
santicomp2014 Jun 12, 2020
aecc382
api/client,api/http,cmd/swarm,storage: changed publisher query param …
santicomp2014 Jun 15, 2020
41cfbe5
api/client: encoded publisher in query param
santicomp2014 Jun 15, 2020
688122a
api/http: added isRecoveryAttempt to List, storage changed recoveryCa…
santicomp2014 Jun 15, 2020
79f3867
api: removed Manifest not found from readManifest, global pinning wil…
santicomp2014 Jun 15, 2020
5e57f81
api/http,storage: changed StatusRecoveryAttempt to 420, refactored Er…
santicomp2014 Jun 16, 2020
3494897
api/http: added isRecoveryAttemptError for HandleGetFile
santicomp2014 Jun 16, 2020
6cc23bf
api/http: changed StatusRecoveryAttempt to StatusEnhanceYourCalm
santicomp2014 Jun 16, 2020
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
13 changes: 1 addition & 12 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,9 +424,6 @@ func (a *API) Get(ctx context.Context, decrypt DecryptFunc, manifestAddr storage
}
mimeType = entry.ContentType
log.Debug("content lookup key", "key", contentAddr, "mimetype", mimeType)
if len(entry.Publisher) > 0 {
ctx = context.WithValue(ctx, "publisher", entry.Publisher)
}
reader, _ = a.fileStore.Retrieve(ctx, contentAddr)
} else {
// no entry found
Expand Down Expand Up @@ -632,12 +629,10 @@ func (a *API) Modify(ctx context.Context, addr storage.Address, path, contentHas
apiModifyFail.Inc(1)
return nil, err
}
publisher, _ := ctx.Value("publisher").(string)
if contentHash != "" {
entry := newManifestTrieEntry(&ManifestEntry{
Path: path,
ContentType: contentType,
Publisher: publisher,
}, nil)
entry.Hash = contentHash
trie.addEntry(entry, quitC)
Expand Down Expand Up @@ -672,14 +667,12 @@ func (a *API) AddFile(ctx context.Context, mhash, path, fname string, content []
path = path[1:]
}

publisher, _ := ctx.Value("publisher").(string)
entry := &ManifestEntry{
Path: filepath.Join(path, fname),
ContentType: mime.TypeByExtension(filepath.Ext(fname)),
Mode: 0700,
Size: int64(len(content)),
ModTime: time.Now(),
Publisher: publisher,
}

mw, err := a.NewManifestWriter(ctx, mkey, nil)
Expand All @@ -704,7 +697,7 @@ func (a *API) AddFile(ctx context.Context, mhash, path, fname string, content []
return fkey, newMkey.String(), nil
}

func (a *API) UploadTar(ctx context.Context, bodyReader io.ReadCloser, manifestPath, defaultPath string, mw *ManifestWriter, publisher string) (storage.Address, error) {
func (a *API) UploadTar(ctx context.Context, bodyReader io.ReadCloser, manifestPath, defaultPath string, mw *ManifestWriter) (storage.Address, error) {
apiUploadTarCount.Inc(1)
var contentKey storage.Address
tr := tar.NewReader(bodyReader)
Expand Down Expand Up @@ -737,7 +730,6 @@ func (a *API) UploadTar(ctx context.Context, bodyReader io.ReadCloser, manifestP
Mode: hdr.Mode,
Size: hdr.Size,
ModTime: hdr.ModTime,
Publisher: publisher,
}
contentKey, err = mw.AddEntry(ctx, tr, entry)
if err != nil {
Expand All @@ -756,7 +748,6 @@ func (a *API) UploadTar(ctx context.Context, bodyReader io.ReadCloser, manifestP
Mode: hdr.Mode,
Size: hdr.Size,
ModTime: hdr.ModTime,
Publisher: publisher,
}
contentKey, err = mw.AddEntry(ctx, nil, entry)
if err != nil {
Expand Down Expand Up @@ -871,14 +862,12 @@ func (a *API) AppendFile(ctx context.Context, mhash, path, fname string, existin
return nil, "", err
}

publisher, _ := ctx.Value("publisher").(string)
entry := &ManifestEntry{
Path: filepath.Join(path, fname),
ContentType: mime.TypeByExtension(filepath.Ext(fname)),
Mode: 0700,
Size: totalSize,
ModTime: time.Now(),
Publisher: publisher,
}

fkey, err := mw.AddEntry(ctx, io.Reader(combinedReader), entry)
Expand Down
31 changes: 20 additions & 11 deletions api/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,11 @@ func Open(path string) (*File, error) {
// (if the manifest argument is non-empty) or creates a new manifest containing
// the file, returning the resulting manifest hash (the file will then be
// available at bzz:/<hash>/<path>)
func (c *Client) Upload(file *File, manifest string, toEncrypt, toPin, anonymous bool, publisher string) (string, error) {
func (c *Client) Upload(file *File, manifest string, toEncrypt, toPin, anonymous bool) (string, error) {
if file.Size <= 0 {
return "", errors.New("file size must be greater than zero")
}
return c.TarUpload(manifest, &FileUploader{file}, "", toEncrypt, toPin, anonymous, publisher)
return c.TarUpload(manifest, &FileUploader{file}, "", toEncrypt, toPin, anonymous)
}

// Download downloads a file with the given path from the swarm manifest with
Expand Down Expand Up @@ -197,7 +197,7 @@ func (c *Client) Download(hash, path string) (*File, error) {
// directory will then be available at bzz:/<hash>/path/to/file), with
// the file specified in defaultPath being uploaded to the root of the manifest
// (i.e. bzz:/<hash>/)
func (c *Client) UploadDirectory(dir, defaultPath, manifest string, toEncrypt, toPin, anonymous bool, publisher string) (string, error) {
func (c *Client) UploadDirectory(dir, defaultPath, manifest string, toEncrypt, toPin, anonymous bool) (string, error) {
stat, err := os.Stat(dir)
if err != nil {
return "", err
Expand All @@ -212,12 +212,12 @@ func (c *Client) UploadDirectory(dir, defaultPath, manifest string, toEncrypt, t
return "", fmt.Errorf("default path: %v", err)
}
}
return c.TarUpload(manifest, &DirectoryUploader{dir}, defaultPath, toEncrypt, toPin, anonymous, publisher)
return c.TarUpload(manifest, &DirectoryUploader{dir}, defaultPath, toEncrypt, toPin, anonymous)
}

// DownloadDirectory downloads the files contained in a swarm manifest under
// the given path into a local directory (existing files will be overwritten)
func (c *Client) DownloadDirectory(hash, path, destDir, credentials string) error {
func (c *Client) DownloadDirectory(hash, path, destDir, credentials, publisher string) error {
stat, err := os.Stat(destDir)
if err != nil {
return err
Expand All @@ -227,6 +227,9 @@ func (c *Client) DownloadDirectory(hash, path, destDir, credentials string) erro

uri := c.Gateway + "/bzz:/" + hash + "/" + path
req, err := http.NewRequest("GET", uri, nil)
values := req.URL.Query()
Copy link
Member

Choose a reason for hiding this comment

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

why not just

 req.URL.Query().Add("publisher", publisher)

Copy link
Contributor Author

@santicomp2014 santicomp2014 Jun 16, 2020

Choose a reason for hiding this comment

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

I used req.URL.Query().Add("publisher", publisher) and the query param was not reaching the other side.
I looked at other places of the code using Add/Set and it was encoding after adding it.

After changing it to .Encode() it works correctly.
@mortelli saw the same thing

values.Add("publisher", publisher)
mortelli marked this conversation as resolved.
Show resolved Hide resolved
req.URL.RawQuery = values.Encode()
if err != nil {
return err
}
Expand Down Expand Up @@ -284,7 +287,7 @@ func (c *Client) DownloadDirectory(hash, path, destDir, credentials string) erro
// DownloadFile downloads a single file into the destination directory
// if the manifest entry does not specify a file name - it will fallback
// to the hash of the file as a filename
func (c *Client) DownloadFile(hash, path, dest, credentials string) error {
func (c *Client) DownloadFile(hash, path, dest, credentials, publisher string) error {
hasDestinationFilename := false
if stat, err := os.Stat(dest); err == nil {
hasDestinationFilename = !stat.IsDir()
Expand All @@ -297,7 +300,7 @@ func (c *Client) DownloadFile(hash, path, dest, credentials string) error {
}
}

manifestList, err := c.List(hash, path, credentials)
manifestList, err := c.List(hash, path, credentials, publisher)
if err != nil {
return err
}
Expand All @@ -313,6 +316,9 @@ func (c *Client) DownloadFile(hash, path, dest, credentials string) error {

uri := c.Gateway + "/bzz:/" + hash + "/" + path
req, err := http.NewRequest("GET", uri, nil)
values := req.URL.Query()
values.Add("publisher", publisher)
mortelli marked this conversation as resolved.
Show resolved Hide resolved
req.URL.RawQuery = values.Encode()
if err != nil {
return err
}
Expand Down Expand Up @@ -410,8 +416,12 @@ func (c *Client) DownloadManifest(hash string) (*api.Manifest, bool, error) {
// - a prefix of "dir1/" would return [dir1/dir2/, dir1/file3.txt]
//
// where entries ending with "/" are common prefixes.
func (c *Client) List(hash, prefix, credentials string) (*api.ManifestList, error) {
req, err := http.NewRequest(http.MethodGet, c.Gateway+"/bzz-list:/"+hash+"/"+prefix, nil)
func (c *Client) List(hash, prefix, credentials, publisher string) (*api.ManifestList, error) {
uri := c.Gateway + "/bzz-list:/" + hash + "/" + prefix
req, err := http.NewRequest(http.MethodGet, uri, nil)
values := req.URL.Query()
values.Add("publisher", publisher)
mortelli marked this conversation as resolved.
Show resolved Hide resolved
req.URL.RawQuery = values.Encode()
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -511,7 +521,7 @@ type UploadFn func(file *File) error

// TarUpload uses the given Uploader to upload files to swarm as a tar stream,
// returning the resulting manifest hash
func (c *Client) TarUpload(hash string, uploader Uploader, defaultPath string, toEncrypt, toPin, anonymous bool, publisher string) (string, error) {
func (c *Client) TarUpload(hash string, uploader Uploader, defaultPath string, toEncrypt, toPin, anonymous bool) (string, error) {
ctx, sp := spancontext.StartSpan(context.Background(), "api.client.tarupload")
defer sp.Finish()

Expand Down Expand Up @@ -539,7 +549,6 @@ func (c *Client) TarUpload(hash string, uploader Uploader, defaultPath string, t
transport := http.DefaultTransport

req.Header.Set("Content-Type", "application/x-tar")
req.Header.Set("publisher", publisher)
if defaultPath != "" {
q := req.URL.Query()
q.Set("defaultpath", defaultPath)
Expand Down
12 changes: 6 additions & 6 deletions api/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func testClientUploadDownloadFiles(toEncrypt bool, t *testing.T) string {
Size: int64(len(data)),
},
}
hash, err := client.Upload(file, manifest, toEncrypt, toPin, true, "")
hash, err := client.Upload(file, manifest, toEncrypt, toPin, true)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -221,7 +221,7 @@ func TestClientUploadDownloadDirectory(t *testing.T) {
// upload the directory
client := NewClient(srv.URL)
defaultPath := testDirFiles[0]
hash, err := client.UploadDirectory(dir, defaultPath, "", false, false, true, "")
hash, err := client.UploadDirectory(dir, defaultPath, "", false, false, true)
if err != nil {
t.Fatalf("error uploading directory: %s", err)
}
Expand Down Expand Up @@ -258,7 +258,7 @@ func TestClientUploadDownloadDirectory(t *testing.T) {
t.Fatal(err)
}
defer os.RemoveAll(tmp)
if err := client.DownloadDirectory(hash, "", tmp, ""); err != nil {
if err := client.DownloadDirectory(hash, "", tmp, "", ""); err != nil {
t.Fatal(err)
}
for _, file := range testDirFiles {
Expand Down Expand Up @@ -289,13 +289,13 @@ func testClientFileList(toEncrypt bool, t *testing.T) {
defer os.RemoveAll(dir)

client := NewClient(srv.URL)
hash, err := client.UploadDirectory(dir, "", "", toEncrypt, false, true, "")
hash, err := client.UploadDirectory(dir, "", "", toEncrypt, false, true)
if err != nil {
t.Fatalf("error uploading directory: %s", err)
}

ls := func(prefix string) []string {
list, err := client.List(hash, prefix, "")
list, err := client.List(hash, prefix, "", "")
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -475,7 +475,7 @@ func TestClientBzzWithFeed(t *testing.T) {
}

// upload data to bzz:// and retrieve the content-addressed manifest hash, hex-encoded.
manifestAddressHex, err := swarmClient.Upload(f, "", false, false, true, "")
manifestAddressHex, err := swarmClient.Upload(f, "", false, false, true)
if err != nil {
t.Fatalf("Error creating manifest: %s", err)
}
Expand Down
35 changes: 28 additions & 7 deletions api/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package http

import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -80,8 +81,9 @@ const (
AnonymousHeaderName = "x-swarm-anonymous" // Presence of this in header indicates only pull sync should be used for upload
PinHeaderName = "x-swarm-pin" // Presence of this in header indicates pinning required

encryptAddr = "encrypt"
tarContentType = "application/x-tar"
encryptAddr = "encrypt"
tarContentType = "application/x-tar"
StatusEnhanceYourCalm = 420
)

type methodHandler map[string]http.Handler
Expand Down Expand Up @@ -239,6 +241,8 @@ type Server struct {

func (s *Server) HandleBzzGet(w http.ResponseWriter, r *http.Request) {
log.Debug("handleBzzGet", "ruid", GetRUID(r.Context()), "uri", r.RequestURI)
publisher := r.URL.Query().Get("publisher")
r = r.WithContext(context.WithValue(r.Context(), "publisher", publisher))
if r.Header.Get("Accept") == tarContentType {
uri := GetURI(r.Context())
_, credentials, _ := r.BasicAuth()
Expand All @@ -249,6 +253,11 @@ func (s *Server) HandleBzzGet(w http.ResponseWriter, r *http.Request) {
respondError(w, r, err.Error(), http.StatusUnauthorized)
return
}
if isRecoveryAttemptError(err) {
w.Header().Set("WWW-Authenticate", fmt.Sprintf("Basic realm=%q", uri.Address().String()))
respondError(w, r, err.Error(), StatusEnhanceYourCalm)
return
}
respondError(w, r, fmt.Sprintf("Had an error building the tarball: %v", err), http.StatusInternalServerError)
return
}
Expand Down Expand Up @@ -468,12 +477,8 @@ func (s *Server) handleTarUpload(r *http.Request, mw *api.ManifestWriter) (stora
log.Debug("handle.tar.upload", "ruid", GetRUID(r.Context()), "tag", sctx.GetTag(r.Context()))

defaultPath := r.URL.Query().Get("defaultpath")
var publisher string
if len(r.Header["Publisher"]) > 0 {
publisher = r.Header["Publisher"][0]
}

key, err := s.api.UploadTar(r.Context(), r.Body, GetURI(r.Context()).Path, defaultPath, mw, publisher)
key, err := s.api.UploadTar(r.Context(), r.Body, GetURI(r.Context()).Path, defaultPath, mw)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -839,6 +844,8 @@ func (s *Server) HandleGet(w http.ResponseWriter, r *http.Request) {
// a list of all files contained in <manifest> under <path> grouped into
// common prefixes using "/" as a delimiter
func (s *Server) HandleGetList(w http.ResponseWriter, r *http.Request) {
publisher := r.URL.Query().Get("publisher")
r = r.WithContext(context.WithValue(r.Context(), "publisher", publisher))
ruid := GetRUID(r.Context())
uri := GetURI(r.Context())
_, credentials, _ := r.BasicAuth()
Expand Down Expand Up @@ -867,6 +874,11 @@ func (s *Server) HandleGetList(w http.ResponseWriter, r *http.Request) {
respondError(w, r, err.Error(), http.StatusUnauthorized)
return
}
if isRecoveryAttemptError(err) {
w.Header().Set("WWW-Authenticate", fmt.Sprintf("Basic realm=%q", addr.String()))
respondError(w, r, err.Error(), StatusEnhanceYourCalm)
return
}
respondError(w, r, err.Error(), http.StatusInternalServerError)
return
}
Expand Down Expand Up @@ -942,6 +954,11 @@ func (s *Server) HandleGetFile(w http.ResponseWriter, r *http.Request) {
respondError(w, r, err.Error(), http.StatusUnauthorized)
return
}
if isRecoveryAttemptError(err) {
w.Header().Set("WWW-Authenticate", fmt.Sprintf("Basic realm=%q", manifestAddr))
respondError(w, r, err.Error(), StatusEnhanceYourCalm)
return
}

switch status {
case http.StatusNotFound:
Expand Down Expand Up @@ -1176,3 +1193,7 @@ func (lrw *loggingResponseWriter) WriteHeader(code int) {
func isDecryptError(err error) bool {
return strings.Contains(err.Error(), api.ErrDecrypt.Error())
}

func isRecoveryAttemptError(err error) bool {
return strings.Contains(err.Error(), storage.ErrRecoveryAttempt.Error())
}
2 changes: 0 additions & 2 deletions api/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ type ManifestEntry struct {
Status int `json:"status,omitempty"`
Access *AccessEntry `json:"access,omitempty"`
Feed *feed.Feed `json:"feed,omitempty"`
Publisher string `json:"publisher,omitempty"`
}

// ManifestList represents the result of listing files in a manifest
Expand Down Expand Up @@ -243,7 +242,6 @@ func readManifest(mr storage.LazySectionReader, addr storage.Address, fileStore
if err != nil { // size == 0
// can't determine size means we don't have the root chunk
log.Trace("manifest not found", "addr", addr)
err = fmt.Errorf("Manifest not Found")
Copy link
Contributor

@mortelli mortelli Jun 16, 2020

Choose a reason for hiding this comment

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

leaving a comment here mainly for @zelig:

the Manifest not Found error created here was swallowing up the 420 that was trying to be returned.

in any case, i believe without line 246 the returned error will be 500 or something of the sort (@santicomp2014 can confirm/refute this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes exactly, the error code becomes Manifest not found and the error code is 500.
The error manifest not found is already being thrown in the error.

return
}
if size > manifestSizeLimit {
Expand Down
2 changes: 1 addition & 1 deletion cmd/swarm-smoke/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func uploadWithTag(data []byte, endpoint string, tag string) (string, error) {
Tag: tag,
}

return swarm.TarUpload("", &client.FileUploader{File: f}, "", false, false, true, "")
return swarm.TarUpload("", &client.FileUploader{File: f}, "", false, false, true)
}

func digest(r io.Reader) ([]byte, error) {
Expand Down
7 changes: 4 additions & 3 deletions cmd/swarm/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
var downloadCommand = cli.Command{
Action: download,
Name: "down",
Flags: []cli.Flag{SwarmRecursiveFlag, SwarmAccessPasswordFlag},
Flags: []cli.Flag{SwarmRecursiveFlag, SwarmAccessPasswordFlag, SwarmPublisher},
Usage: "downloads a swarm manifest or a file inside a manifest",
ArgsUsage: " <uri> [<dir>]",
Description: `Downloads a swarm bzz uri to the given dir. When no dir is provided, working directory is assumed. --recursive flag is expected when downloading a manifest with multiple entries.`,
Expand Down Expand Up @@ -60,6 +60,7 @@ func download(ctx *cli.Context) {
bzzapi = strings.TrimRight(ctx.GlobalString(SwarmApiFlag.Name), "/")
isRecursive = ctx.Bool(SwarmRecursiveFlag.Name)
client = swarm.NewClient(bzzapi)
publisher = ctx.String(SwarmPublisher.Name)
)

if fi, err := os.Stat(dest); err == nil {
Expand All @@ -80,7 +81,7 @@ func download(ctx *cli.Context) {
dl := func(credentials string) error {
// assume behaviour according to --recursive switch
if isRecursive {
if err := client.DownloadDirectory(uri.Addr, uri.Path, dest, credentials); err != nil {
if err := client.DownloadDirectory(uri.Addr, uri.Path, dest, credentials, publisher); err != nil {
if err == swarm.ErrUnauthorized {
return err
}
Expand All @@ -90,7 +91,7 @@ func download(ctx *cli.Context) {
// we are downloading a file
log.Debug("downloading file/path from a manifest", "uri.Addr", uri.Addr, "uri.Path", uri.Path)

err := client.DownloadFile(uri.Addr, uri.Path, dest, credentials)
err := client.DownloadFile(uri.Addr, uri.Path, dest, credentials, publisher)
if err != nil {
if err == swarm.ErrUnauthorized {
return err
Expand Down
Loading