Implement the get charm file API.

state/apiserver/charms.go

 // Copyright 2013 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package apiserver
 
 import (
         "archive/zip"
         "crypto/sha256"
         "encoding/base64"
         "encoding/hex"
@@ skipping 104 matching lines @@
         }
         h.sendJSON(w, http.StatusOK, &params.CharmsResponse{Files: files})
 }
 
 // fileSender returns a zipContentsSenderFunc which is responsible of sending
 // the contents of filePath included in the given zip.
 // A 404 page not found is returned if path does not exist in the zip.
 // A 403 forbidden error is returned if path points to a directory.
 func (h *charmsHandler) fileSender(filePath string) zipContentsSenderFunc {
         return func(w http.ResponseWriter, r *http.Request, reader *zip.ReadCloser) {
                 for _, file := range reader.File {

[dimitern, 2014/02/25 17:40:03]

gofmt again?

[frankban, 2014/02/26 09:34:28]

Done, and it seems already ok.

                         if h.fixPath(file.Name) != filePath {
                                 continue
                         }
                         fileInfo := file.FileInfo()
                         if fileInfo.IsDir() {
                                 http.Error(w, "directory listing not allowed", http.StatusForbidden)
                                 return
                         }
                         if contents, err := file.Open(); err != nil {
                                 http.Error(
                                         w, fmt.Sprintf("unable to read file %q: %v", filePath, err),
                                         http.StatusInternalServerError)
+                                return
                         } else {

[dimitern, 2014/02/25 17:40:03]

Instead of else, you could just return in the if block above.

[frankban, 2014/02/26 09:34:28]

We need contents in the scope, that's why I used else.

[dimitern, 2014/02/26 10:01:03]

Then you can just move the assignment out of the if.

                                 defer contents.Close()
                                 ctype := mime.TypeByExtension(filepath.Ext(filePath))
                                 if ctype != "" {
                                         w.Header().Set("Content-Type", ctype)
                                 }
                                 w.Header().Set("Content-Length", strconv.FormatInt(fileInfo.Size(), 10))
                                 w.WriteHeader(http.StatusOK)
                                 io.Copy(w, contents)
                         }
                         return
@@ skipping 352 matching lines @@
         }
 
         // And finally, update state.
         _, err = h.state.UpdateUploadedCharm(archive, curl, bundleURL, bundleSHA256)
         if err != nil {
                 return errgo.Annotate(err, "cannot update uploaded charm in state")
         }
         return nil
 }
 
-// processGet handles a charm file download GET request after authentication.

[dimitern, 2014/02/25 17:40:03]

s/download//

[frankban, 2014/02/26 09:34:28]

Done.

+// processGet handles a charm file GET request after authentication.
 // It returns the bundle path, the requested file path (if any) and an error.
 func (h *charmsHandler) processGet(r *http.Request) (string, string, error) {
         query := r.URL.Query()
 
         // Retrieve and validate query parameters.
         curl := query.Get("url")
         if curl == "" {
                 return "", "", fmt.Errorf("expected url=CharmURL query argument")
         }
         var filePath string
@@ skipping 14 matching lines @@
                 if err = h.downloadCharm(name, charmArchivePath); err != nil {
                         return "", "", fmt.Errorf("unable to retrieve and save the charm: %v", err)
                 }
         } else if err != nil {
                 return "", "", fmt.Errorf("cannot access the charms cache: %v", err)
         }
         return charmArchivePath, filePath, nil
 }
 
 // downloadCharm downloads the given charm name from the provider storage and
-// save the corresponding zip archive to the given charmArchivePath.

[dimitern, 2014/02/25 17:40:03]

s/save/saves/

[frankban, 2014/02/26 09:34:28]

Done.

+// saves the corresponding zip archive to the given charmArchivePath.
 func (h *charmsHandler) downloadCharm(name, charmArchivePath string) error {
         // Get the provider storage.
         storage, err := envtesting.GetEnvironStorage(h.state)
         if err != nil {
                 return errgo.Annotate(err, "cannot access provider storage")
         }
 
         // Use the storage to retrieve and save the charm archive.
         reader, err := storage.Get(name)
         if err != nil {
@@ skipping 13 matching lines @@
                 return errgo.Annotate(err, "cannot create the charms cache")
         }
         tempCharmArchive, err := ioutil.TempFile(cacheDir, "charm")
         if err != nil {
                 return errgo.Annotate(err, "cannot create charm archive temp file")
         }
         defer tempCharmArchive.Close()
         if err = ioutil.WriteFile(tempCharmArchive.Name(), data, 0644); err != nil {
                 return errgo.Annotate(err, "error processing charm archive download")
         }
         if err = os.Rename(tempCharmArchive.Name(), charmArchivePath); err != nil {

[dimitern, 2014/02/25 17:40:03]

In this case I think you should defer os.Remove(tempCharmArchive.Name()) here.

[frankban, 2014/02/26 09:34:28]

Good catch, fixed.

+                defer os.Remove(tempCharmArchive.Name())
                 return errgo.Annotate(err, "error renaming the charm archive")
         }
         return nil
 }