Manifest cli fix and upload defaultpath only once (#17375)

* cmd/swarm: fix manifest subcommands and add tests

* cmd/swarm: manifest update: update default entry for non-encrypted uploads

* swarm/api: upload defaultpath file only once

* swarm/api/client: improve UploadDirectory default path handling

* cmd/swarm: support absolute and relative default path values

* cmd/swarm: fix a typo in test

* cmd/swarm: check encrypted uploads in manifest update tests

cmd/swarm/main.go

@@ -322,23 +322,23 @@ Downloads a swarm bzz uri to the given dir. When no dir is provided, working dir
 			Description:        "Updates a MANIFEST by adding/removing/updating the hash of a path.\nCOMMAND could be: add, update, remove",
 			Subcommands: []cli.Command{
 				{
-					Action:             add,
+					Action:             manifestAdd,
 					CustomHelpTemplate: helpTemplate,
 					Name:               "add",
 					Usage:              "add a new path to the manifest",
-					ArgsUsage:          "<MANIFEST> <path> <hash> [<content-type>]",
+					ArgsUsage:          "<MANIFEST> <path> <hash>",
 					Description:        "Adds a new path to the manifest",
 				},
 				{
-					Action:             update,
+					Action:             manifestUpdate,
 					CustomHelpTemplate: helpTemplate,
 					Name:               "update",
 					Usage:              "update the hash for an already existing path in the manifest",
-					ArgsUsage:          "<MANIFEST> <path> <newhash> [<newcontent-type>]",
+					ArgsUsage:          "<MANIFEST> <path> <newhash>",
 					Description:        "Update the hash for an already existing path in the manifest",
 				},
 				{
-					Action:             remove,
+					Action:             manifestRemove,
 					CustomHelpTemplate: helpTemplate,
 					Name:               "remove",
 					Usage:              "removes a path from the manifest",

cmd/swarm/upload.go

@@ -98,6 +98,17 @@ func upload(ctx *cli.Context) {
 			if !recursive {
 				return "", errors.New("Argument is a directory and recursive upload is disabled")
 			}
+			if defaultPath != "" {
+				// construct absolute default path
+				absDefaultPath, _ := filepath.Abs(defaultPath)
+				absFile, _ := filepath.Abs(file)
+				// make sure absolute directory ends with only one "/"
+				// to trim it from absolute default path and get relative default path
+				absFile = strings.TrimRight(absFile, "/") + "/"
+				if absDefaultPath != "" && absFile != "" && strings.HasPrefix(absDefaultPath, absFile) {
+					defaultPath = strings.TrimPrefix(absDefaultPath, absFile)
+				}
+			}
 			return client.UploadDirectory(file, defaultPath, "", toEncrypt)
 		}
 	} else {

cmd/swarm/upload_test.go

@@ -273,3 +273,84 @@ func testCLISwarmUpRecursive(toEncrypt bool, t *testing.T) {
 		}
 	}
 }
+
+// TestCLISwarmUpDefaultPath tests swarm recursive upload with relative and absolute
+// default paths and with encryption.
+func TestCLISwarmUpDefaultPath(t *testing.T) {
+	testCLISwarmUpDefaultPath(false, false, t)
+	testCLISwarmUpDefaultPath(false, true, t)
+	testCLISwarmUpDefaultPath(true, false, t)
+	testCLISwarmUpDefaultPath(true, true, t)
+}
+
+func testCLISwarmUpDefaultPath(toEncrypt bool, absDefaultPath bool, t *testing.T) {
+	cluster := newTestCluster(t, 1)
+	defer cluster.Shutdown()
+
+	tmp, err := ioutil.TempDir("", "swarm-defaultpath-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmp)
+
+	err = ioutil.WriteFile(filepath.Join(tmp, "index.html"), []byte("<h1>Test</h1>"), 0666)
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = ioutil.WriteFile(filepath.Join(tmp, "robots.txt"), []byte("Disallow: /"), 0666)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defaultPath := "index.html"
+	if absDefaultPath {
+		defaultPath = filepath.Join(tmp, defaultPath)
+	}
+
+	args := []string{
+		"--bzzapi",
+		cluster.Nodes[0].URL,
+		"--recursive",
+		"--defaultpath",
+		defaultPath,
+		"up",
+		tmp,
+	}
+	if toEncrypt {
+		args = append(args, "--encrypt")
+	}
+
+	up := runSwarm(t, args...)
+	hashRegexp := `[a-f\d]{64,128}`
+	_, matches := up.ExpectRegexp(hashRegexp)
+	up.ExpectExit()
+	hash := matches[0]
+
+	client := swarm.NewClient(cluster.Nodes[0].URL)
+
+	m, isEncrypted, err := client.DownloadManifest(hash)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if toEncrypt != isEncrypted {
+		t.Error("downloaded manifest is not encrypted")
+	}
+
+	var found bool
+	var entriesCount int
+	for _, e := range m.Entries {
+		entriesCount++
+		if e.Path == "" {
+			found = true
+		}
+	}
+
+	if !found {
+		t.Error("manifest default entry was not found")
+	}
+
+	if entriesCount != 3 {
+		t.Errorf("manifest contains %v entries, expected %v", entriesCount, 3)
+	}
+}

swarm/api/api.go

@@ -704,11 +704,12 @@ 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 string, mw *ManifestWriter) (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)
 	defer bodyReader.Close()
+	var defaultPathFound bool
 	for {
 		hdr, err := tr.Next()
 		if err == io.EOF {
@@ -737,6 +738,25 @@ func (a *API) UploadTar(ctx context.Context, bodyReader io.ReadCloser, manifestP
 			apiUploadTarFail.Inc(1)
 			return nil, fmt.Errorf("error adding manifest entry from tar stream: %s", err)
 		}
+		if hdr.Name == defaultPath {
+			entry := &ManifestEntry{
+				Hash:        contentKey.Hex(),
+				Path:        "", // default entry
+				ContentType: hdr.Xattrs["user.swarm.content-type"],
+				Mode:        hdr.Mode,
+				Size:        hdr.Size,
+				ModTime:     hdr.ModTime,
+			}
+			contentKey, err = mw.AddEntry(ctx, nil, entry)
+			if err != nil {
+				apiUploadTarFail.Inc(1)
+				return nil, fmt.Errorf("error adding default manifest entry from tar stream: %s", err)
+			}
+			defaultPathFound = true
+		}
+	}
+	if defaultPath != "" && !defaultPathFound {
+		return contentKey, fmt.Errorf("default path %q not found", defaultPath)
 	}
 	return contentKey, nil
 }

swarm/api/client/client.go

@@ -138,7 +138,7 @@ func (c *Client) Upload(file *File, manifest string, toEncrypt bool) (string, er
 	if file.Size <= 0 {
 		return "", errors.New("file size must be greater than zero")
 	}
-	return c.TarUpload(manifest, &FileUploader{file}, toEncrypt)
+	return c.TarUpload(manifest, &FileUploader{file}, "", toEncrypt)
 }
 
 // Download downloads a file with the given path from the swarm manifest with
@@ -175,7 +175,15 @@ func (c *Client) UploadDirectory(dir, defaultPath, manifest string, toEncrypt bo
 	} else if !stat.IsDir() {
 		return "", fmt.Errorf("not a directory: %s", dir)
 	}
-	return c.TarUpload(manifest, &DirectoryUploader{dir, defaultPath}, toEncrypt)
+	if defaultPath != "" {
+		if _, err := os.Stat(filepath.Join(dir, defaultPath)); err != nil {
+			if os.IsNotExist(err) {
+				return "", fmt.Errorf("the default path %q was not found in the upload directory %q", defaultPath, dir)
+			}
+			return "", fmt.Errorf("default path: %v", err)
+		}
+	}
+	return c.TarUpload(manifest, &DirectoryUploader{dir}, defaultPath, toEncrypt)
 }
 
 // DownloadDirectory downloads the files contained in a swarm manifest under
@@ -389,21 +397,11 @@ func (u UploaderFunc) Upload(upload UploadFn) error {
 // DirectoryUploader uploads all files in a directory, optionally uploading
 // a file to the default path
 type DirectoryUploader struct {
-	Dir         string
-	DefaultPath string
+	Dir string
 }
 
 // Upload performs the upload of the directory and default path
 func (d *DirectoryUploader) Upload(upload UploadFn) error {
-	if d.DefaultPath != "" {
-		file, err := Open(d.DefaultPath)
-		if err != nil {
-			return err
-		}
-		if err := upload(file); err != nil {
-			return err
-		}
-	}
 	return filepath.Walk(d.Dir, func(path string, f os.FileInfo, err error) error {
 		if err != nil {
 			return err
@@ -441,7 +439,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, toEncrypt bool) (string, error) {
+func (c *Client) TarUpload(hash string, uploader Uploader, defaultPath string, toEncrypt bool) (string, error) {
 	reqR, reqW := io.Pipe()
 	defer reqR.Close()
 	addr := hash
@@ -458,6 +456,11 @@ func (c *Client) TarUpload(hash string, uploader Uploader, toEncrypt bool) (stri
 		return "", err
 	}
 	req.Header.Set("Content-Type", "application/x-tar")
+	if defaultPath != "" {
+		q := req.URL.Query()
+		q.Set("defaultpath", defaultPath)
+		req.URL.RawQuery = q.Encode()
+	}
 
 	// use 'Expect: 100-continue' so we don't send the request body if
 	// the server refuses the request

swarm/api/client/client_test.go

@@ -194,7 +194,7 @@ func TestClientUploadDownloadDirectory(t *testing.T) {
 
 	// upload the directory
 	client := NewClient(srv.URL)
-	defaultPath := filepath.Join(dir, testDirFiles[0])
+	defaultPath := testDirFiles[0]
 	hash, err := client.UploadDirectory(dir, defaultPath, "", false)
 	if err != nil {
 		t.Fatalf("error uploading directory: %s", err)

swarm/api/http/server.go

@@ -336,7 +336,9 @@ func (s *Server) HandlePostFiles(w http.ResponseWriter, r *http.Request) {
 func (s *Server) handleTarUpload(r *http.Request, mw *api.ManifestWriter) (storage.Address, error) {
 	log.Debug("handle.tar.upload", "ruid", GetRUID(r.Context()))
 
-	key, err := s.api.UploadTar(r.Context(), r.Body, GetURI(r.Context()).Path, mw)
+	defaultPath := r.URL.Query().Get("defaultpath")
+
+	key, err := s.api.UploadTar(r.Context(), r.Body, GetURI(r.Context()).Path, defaultPath, mw)
 	if err != nil {
 		return nil, err
 	}

swarm/api/manifest.go

@@ -106,13 +106,18 @@ func (a *API) NewManifestWriter(ctx context.Context, addr storage.Address, quitC
 }
 
 // AddEntry stores the given data and adds the resulting key to the manifest
-func (m *ManifestWriter) AddEntry(ctx context.Context, data io.Reader, e *ManifestEntry) (storage.Address, error) {
-	key, _, err := m.api.Store(ctx, data, e.Size, m.trie.encrypted)
-	if err != nil {
-		return nil, err
-	}
+func (m *ManifestWriter) AddEntry(ctx context.Context, data io.Reader, e *ManifestEntry) (key storage.Address, err error) {
 	entry := newManifestTrieEntry(e, nil)
-	entry.Hash = key.Hex()
+	if data != nil {
+		key, _, err = m.api.Store(ctx, data, e.Size, m.trie.encrypted)
+		if err != nil {
+			return nil, err
+		}
+		entry.Hash = key.Hex()
+	}
+	if entry.Hash == "" {
+		return key, errors.New("missing entry hash")
+	}
 	m.trie.addEntry(entry, m.quitC)
 	return key, nil
 }