From 6dc859247ff8d71cbce5eda908555cec8aadb451 Mon Sep 17 00:00:00 2001
From: Fabrice Matrat <fabricematrat@gmail.com>
Date: Tue, 24 Jul 2018 09:36:27 +0200
Subject: [PATCH] Reviews.

---
 internal/charmstore/archive.go | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/internal/charmstore/archive.go b/internal/charmstore/archive.go
index 61e329f4c..a548a66ca 100644
--- a/internal/charmstore/archive.go
+++ b/internal/charmstore/archive.go
@@ -115,6 +115,9 @@ func (r *multiReadSeekCloser) Close() error {
 // If no such file was found, it returns an error
 // with a params.ErrNotFound cause.
 //
+// If the file is a symlink, it returns an error ErrRedirect that holds the path
+// that the symbolic link refers to.
+//
 // If the file is actually a directory in the blob, it returns
 // an error with a params.ErrForbidden cause.
 func (s *Store) OpenBlobFile(blob *Blob, filePath string) (io.ReadCloser, int64, error) {
@@ -137,25 +140,28 @@ func (s *Store) OpenBlobFile(blob *Blob, filePath string) (io.ReadCloser, int64,
 		if err != nil {
 			return nil, 0, errgo.Notef(err, "unable to read file %q", filePath)
 		}
-		if file.Mode()&os.ModeSymlink != 0 {
-			defer content.Close()
-			url, err := ioutil.ReadAll(content)
-			if err != nil {
-				return nil, 0, errgo.Notef(err, "cannot read archive data for symlink %s", file.Name)
-			}
-			return nil, 0, ErrRedirect{URL: string(url)}
+		if file.Mode()&os.ModeSymlink == 0 {
+			return content, fileInfo.Size(), nil
+		}
+		defer content.Close()
+		url, err := ioutil.ReadAll(content)
+		if err != nil {
+			return nil, 0, errgo.Notef(err, "cannot read archive data for symlink %s", file.Name)
 		}
-		return content, fileInfo.Size(), nil
+		return nil, 0, &ErrRedirect{Path: string(url)}
 	}
 	return nil, 0, errgo.WithCausef(nil, params.ErrNotFound, "file %q not found in the archive", filePath)
 }
 
+// ErrRedirect holds the error returned by OpenBlobFile when a symbolic link
+// is opened. The Path field holds the name of the file that the symbolic link
+// refers to.
 type ErrRedirect struct {
-	URL string
+	Path string
 }
 
 func (e ErrRedirect) Error() string {
-	return fmt.Sprintf("redirect to %v", e.URL)
+	return fmt.Sprintf("file is a symbolic link to %q", e.Path)
 }
 
 // OpenCachedBlobFile opens a file from the given entity's archive blob.