cmd/swarm, swarm/api: Use common method to detect Content-Type (ether…

…sphere#527, ethersphere#944)

Changed swarm/api.Upload:
 - when using WaitGroup no reason to use done counter.
 - f.Close() must be called in Defer - otherwise panic or future added early return will cause leak of file descriptors
 - use common DetectContentType method
 - one error was suppressed

cmd/swarm/upload.go

@@ -22,16 +22,14 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"mime"
-	"net/http"
 	"os"
 	"os/user"
 	"path"
 	"path/filepath"
 	"strings"
 
 	"github.com/ethereum/go-ethereum/cmd/utils"
-	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/swarm/api"
 	swarm "github.com/ethereum/go-ethereum/swarm/api/client"
 	"gopkg.in/urfave/cli.v1"
 )
@@ -120,7 +118,7 @@ func upload(ctx *cli.Context) {
 			}
 			defer f.Close()
 			if mimeType == "" {
-				mimeType = detectMimeType(file)
+				mimeType = api.DetectContentType(file)
 			}
 			f.ContentType = mimeType
 			return client.Upload(f, "", toEncrypt)
@@ -156,27 +154,3 @@ func homeDir() string {
 	}
 	return ""
 }
-
-// try detect by extension as it's cheaper
-// otherwise detect by file content
-// if it cannot determine a more specific one, it
-// returns "application/octet-stream".
-func detectMimeType(file string) string {
-	if ext := filepath.Ext(file); ext != "" {
-		if mimeType := mime.TypeByExtension(ext); mimeType != "" {
-			return mimeType
-		}
-	}
-
-	f, err := os.Open(file)
-	if err != nil {
-		log.Warn("detectMimeType: can't open file", "file", file, "err", err)
-		return "application/octet-stream"
-	}
-	defer f.Close()
-	buf := make([]byte, 512)
-	if n, _ := f.Read(buf); n > 0 {
-		return http.DetectContentType(buf)
-	}
-	return "application/octet-stream"
-}

cmd/swarm/upload_test.go

@@ -32,7 +32,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/log"
 	swarm "github.com/ethereum/go-ethereum/swarm/api/client"
-	colorable "github.com/mattn/go-colorable"
+	"github.com/mattn/go-colorable"
 )
 
 var loglevel = flag.Int("loglevel", 3, "verbosity of logs")
@@ -354,18 +354,3 @@ func testCLISwarmUpDefaultPath(toEncrypt bool, absDefaultPath bool, t *testing.T
 		t.Errorf("manifest contains %v entries, expected %v", entriesCount, 3)
 	}
 }
-
-func TestDetectMimeTypeRequireFallbackToOctetStream(t *testing.T) {
-	for pathToFile, expect := range map[string]string{
-		"./path/to/file.pdf":    "application/pdf",
-		"./path/to/file.md":     "application/octet-stream",
-		"":                      "application/octet-stream",
-		"noextension":           "application/octet-stream",
-		"./path/to/noextension": "application/octet-stream",
-	} {
-		detected := detectMimeType(pathToFile)
-		if detected != expect {
-			t.Fatalf("Expected mime type %s, got %s", expect, detected)
-		}
-	}
-}

swarm/api/content_type.go

@@ -0,0 +1,34 @@
+package api
+
+import (
+	"mime"
+	"net/http"
+	"os"
+	"path/filepath"
+
+	"github.com/ethereum/go-ethereum/log"
+)
+
+// detect first by file extension
+// otherwise detect by file content
+// if it cannot determine a more specific one, it
+// returns "application/octet-stream".
+func DetectContentType(file string) string {
+	if ext := filepath.Ext(file); ext != "" {
+		if mimeType := mime.TypeByExtension(ext); mimeType != "" {
+			return mimeType
+		}
+	}
+
+	f, err := os.Open(file)
+	if err != nil {
+		log.Warn("detectMimeType: can't open file", "file", file, "err", err)
+		return "application/octet-stream"
+	}
+	defer f.Close()
+	buf := make([]byte, 512)
+	if n, _ := f.Read(buf); n > 0 {
+		return http.DetectContentType(buf)
+	}
+	return "application/octet-stream"
+}

swarm/api/content_type_test.go

@@ -0,0 +1,40 @@
+package api
+
+import "testing"
+
+func TestDetectContentType(t *testing.T) {
+	for _, v := range []struct {
+		path                string
+		expectedContentType string
+	}{
+		{
+			path:                "./path/to/file.pdf",
+			expectedContentType: "application/pdf",
+		},
+		{
+			path:                "./path/to/file.md",
+			expectedContentType: "application/octet-stream",
+		},
+		{
+			path:                "",
+			expectedContentType: "application/octet-stream",
+		},
+		{
+			path:                "noextension",
+			expectedContentType: "application/octet-stream",
+		},
+		{
+			path:                "./path/to/noextension",
+			expectedContentType: "application/octet-stream",
+		},
+		{
+			path:                "./1.css",
+			expectedContentType: "text/css; charset=utf-8",
+		},
+	} {
+		detected := DetectContentType(v.path)
+		if detected != v.expectedContentType {
+			t.Fatalf("Expected mime type %s, got %s", v.expectedContentType, detected)
+		}
+	}
+}

swarm/api/filesystem.go

@@ -21,7 +21,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net/http"
 	"os"
 	"path"
 	"path/filepath"
@@ -99,51 +98,46 @@ func (fs *FileSystem) Upload(lpath, index string, toEncrypt bool) (string, error
 
 	cnt := len(list)
 	errors := make([]error, cnt)
-	done := make(chan bool, maxParallelFiles)
-	dcnt := 0
 	awg := &sync.WaitGroup{}
 
 	for i, entry := range list {
-		if i >= dcnt+maxParallelFiles {
-			<-done
-			dcnt++
-		}
 		awg.Add(1)
-		go func(i int, entry *manifestTrieEntry, done chan bool) {
+		go func(i int, entry *manifestTrieEntry) {
+			defer awg.Done()
+
 			f, err := os.Open(entry.Path)
-			if err == nil {
-				stat, _ := f.Stat()
-				var hash storage.Address
-				var wait func(context.Context) error
-				ctx := context.TODO()
-				hash, wait, err = fs.api.fileStore.Store(ctx, f, stat.Size(), toEncrypt)
-				if hash != nil {
-					list[i].Hash = hash.Hex()
-				}
-				err = wait(ctx)
-				awg.Done()
-				if err == nil {
-					first512 := make([]byte, 512)
-					fread, _ := f.ReadAt(first512, 0)
-					if fread > 0 {
-						mimeType := http.DetectContentType(first512[:fread])
-						if filepath.Ext(entry.Path) == ".css" {
-							mimeType = "text/css"
-						}
-						list[i].ContentType = mimeType
-					}
-				}
-				f.Close()
+			if err != nil {
+				errors[i] = err
+				return
 			}
-			errors[i] = err
-			done <- true
-		}(i, entry, done)
-	}
-	for dcnt < cnt {
-		<-done
-		dcnt++
+			defer f.Close()
+
+			stat, err := f.Stat()
+			if err != nil {
+				errors[i] = err
+				return
+			}
+
+			var hash storage.Address
+			var wait func(context.Context) error
+			ctx := context.TODO()
+			hash, wait, err = fs.api.fileStore.Store(ctx, f, stat.Size(), toEncrypt)
+			if hash != nil {
+				list[i].Hash = hash.Hex()
+			}
+
+			list[i].ContentType = DetectContentType(entry.Path)
+
+			if err := wait(ctx); err != nil {
+				errors[i] = err
+				return
+			}
+
+		}(i, entry)
 	}
 
+	awg.Wait()
+
 	trie := &manifestTrie{
 		fileStore: fs.api.fileStore,
 	}
@@ -168,7 +162,6 @@ func (fs *FileSystem) Upload(lpath, index string, toEncrypt bool) (string, error
 	if err2 == nil {
 		hs = trie.ref.Hex()
 	}
-	awg.Wait()
 	return hs, err2
 }
 

swarm/api/filesystem_test.go

@@ -60,7 +60,7 @@ func TestApiDirUpload0(t *testing.T) {
 
 		content = readPath(t, "testdata", "test0", "index.css")
 		resp = testGet(t, api, bzzhash, "index.css")
-		exp = expResponse(content, "text/css", 0)
+		exp = expResponse(content, "text/css; charset=utf-8", 0)
 		checkResponse(t, resp, exp)
 
 		addr := storage.Address(common.Hex2Bytes(bzzhash))
@@ -140,7 +140,7 @@ func TestApiDirUploadModify(t *testing.T) {
 
 		content = readPath(t, "testdata", "test0", "index.css")
 		resp = testGet(t, api, bzzhash, "index.css")
-		exp = expResponse(content, "text/css", 0)
+		exp = expResponse(content, "text/css; charset=utf-8", 0)
 		checkResponse(t, resp, exp)
 
 		_, _, _, _, err = api.Get(context.TODO(), nil, addr, "")