store: add functions to download snap icons

store/download_test.go

@@ -708,3 +708,109 @@ func (s *downloadSuite) TestActualDownloadRateLimited(c *C) {
 	c.Check(buf.String(), Equals, canary)
 	c.Check(ratelimitReaderUsed, Equals, true)
 }
+
+func (s *downloadSuite) TestActualDownloadIcon(c *C) {
+	n := 0
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		c.Check(r.Header.Get("Snap-CDN"), Equals, "")
+		c.Check(r.Header.Get("Snap-Device-Location"), Equals, "")
+		c.Check(r.Header.Get("Snap-Refresh-Reason"), Equals, "")
+		n++
+		io.WriteString(w, "response-data")
+	}))
+	c.Assert(mockServer, NotNil)
+	defer mockServer.Close()
+
+	var buf SillyBuffer
+	err := store.DownloadIconImpl(context.TODO(), "foo", mockServer.URL, &buf)
+	c.Assert(err, IsNil)
+	c.Check(buf.String(), Equals, "response-data")
+	c.Check(n, Equals, 1)
+}
+
+func (s *downloadSuite) TestDownloadIconCancellation(c *C) {
+	// the channel used by mock server to request cancellation from the test
+	syncCh := make(chan struct{})
+
+	n := 0
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		n++
+		io.WriteString(w, "foo")
+		syncCh <- struct{}{}
+		io.WriteString(w, "bar")
+		time.Sleep(10 * time.Millisecond)
+	}))
+	c.Assert(mockServer, NotNil)
+	defer mockServer.Close()
+
+	ctx, cancel := context.WithCancel(context.Background())
+
+	result := make(chan string)
+	go func() {
+		var buf SillyBuffer
+		err := store.DownloadIconImpl(ctx, "foo", mockServer.URL, &buf)
+		result <- err.Error()
+		close(result)
+	}()
+
+	<-syncCh
+	cancel()
+
+	err := <-result
+	c.Check(n, Equals, 1)
+	c.Assert(err, Equals, "the download has been cancelled: context canceled")
+}
+
+func (s *downloadSuite) TestActualDownloadIcon404(c *C) {
+	n := 0
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		n++
+		w.WriteHeader(404)
+	}))
+	c.Assert(mockServer, NotNil)
+	defer mockServer.Close()
+
+	var buf SillyBuffer
+	err := store.DownloadIconImpl(context.TODO(), "foo", mockServer.URL, &buf)
+	c.Assert(err, NotNil)
+	c.Assert(err, FitsTypeOf, &store.DownloadError{})
+	c.Check(err.(*store.DownloadError).Code, Equals, 404)
+	c.Check(n, Equals, 1)
+}
+
+func (s *downloadSuite) TestActualDownloadIcon500(c *C) {
+	n := 0
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		n++
+		w.WriteHeader(500)
+	}))
+	c.Assert(mockServer, NotNil)
+	defer mockServer.Close()
+
+	var buf SillyBuffer
+	err := store.DownloadIconImpl(context.TODO(), "foo", mockServer.URL, &buf)
+	c.Assert(err, NotNil)
+	c.Assert(err, FitsTypeOf, &store.DownloadError{})
+	c.Check(err.(*store.DownloadError).Code, Equals, 500)
+	c.Check(n, Equals, 5)
+}
+
+func (s *downloadSuite) TestActualDownloadIcon500Once(c *C) {
+	n := 0
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		n++
+		if n == 1 {
+			w.WriteHeader(500)
+		} else {
+			io.WriteString(w, "response-data")
+		}
+	}))
+	c.Assert(mockServer, NotNil)
+	defer mockServer.Close()
+
+	var buf SillyBuffer
+	err := store.DownloadIconImpl(context.TODO(), "foo", mockServer.URL, &buf)
+	c.Assert(err, IsNil)
+	c.Check(buf.String(), Equals, "response-data")
+	c.Check(n, Equals, 2)
+}

store/export_test.go

@@ -42,6 +42,8 @@ var (
 	ApiURL        = apiURL
 	Download      = download
 
+	DownloadIconImpl = downloadIcon
+
 	ApplyDelta = applyDelta
 
 	AuthLocation      = authLocation
@@ -149,6 +151,10 @@ func MockDownload(f func(ctx context.Context, name, sha3_384, downloadURL string
 	}
 }
 
+func MockDownloadIcon(f func(ctx context.Context, name, downloadURL string, w io.ReadWriteSeeker) error) (restore func()) {
+	return testutil.Mock(&downloadIcon, f)
+}
+
 func MockDoDownloadReq(f func(ctx context.Context, storeURL *url.URL, cdnHeader string, resume int64, s *Store, user *auth.UserState) (*http.Response, error)) (restore func()) {
 	orig := doDownloadReq
 	doDownloadReq = f

store/store.go

@@ -766,6 +766,22 @@
 	}
 }
 
+// doIconRequest does an unauthenticated GET request to the given URL.
+func doIconRequest(ctx context.Context, client *http.Client, iconURL *url.URL) (*http.Response, error) {
+	var body io.Reader // empty body
+	req, err := http.NewRequestWithContext(ctx, "GET", iconURL.String(), body)
+	if err != nil {
+		return nil, err
+	}
+
+	resp, err := client.Do(req)
+	if err != nil {
+		return nil, err
+	}
+
+	return resp, err
+}
+
 func (s *Store) buildLocationString() (string, error) {
 	if s.dauthCtx == nil {
 		return "", nil

store/store_download.go

@@ -308,6 +308,65 @@
 	return s.cacher.Put(downloadInfo.Sha3_384, targetPath)
 }
 
+// DownloadIcon downloads the icon for the snap from the given download URL to
+// the given target path. Snap icons are small (<256kB) files served from an
+// ordinary unauthenticated file server, so this does not require store
+// authentication or user state, nor a progress bar. They are also not revision-
+// specific, and do not use a download cache.
+func DownloadIcon(ctx context.Context, name string, targetPath string, downloadURL string) error {
+	if err := os.MkdirAll(filepath.Dir(targetPath), 0o755); err != nil {
+		return err
+	}
+
+	// Download icon to a temporary file location, since there may be an active
+	// snap icon hard-linked to targetPath. If download fails, don't want to
+	// corrupt the existing icon file contents. Instead, move partialPath to
+	// targetPath once the download succeeds, thus leaving any previous icon
+	// linked to the original unchanged icon contents, until a future task
+	// re-links to the new contents.
+	partialPath := targetPath + ".partial"
+
+	// Open the new partial file and truncate it, since any existing file may
+	// be old, from a previous version of the snap icon. Don't use `resume` as
+	// in `Download()`; always start over.
+	w, err := os.OpenFile(partialPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o600)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		// Unconditionally close the file, whether it's been closed before or
+		// not. If it has been closed before, close will return an error, which
+		// we ignore. If it hasn't been called before, then an error has already
+		// occurred, and we don't care about any new error from calling Close().
+		w.Close()
+		// Unconditionally remove the .partial file, since w.Name() is the name
+		// with which the file was originally opened. If it still exists, then
+		// an error occurred, and we want to remove it. If it doesn't still
+		// exist, then it was successfully renamed, so it's harmless to attempt
+		// to remove the original .partial filename, and we ignore the error.
+		os.Remove(w.Name())
+	}()
+	logger.Debugf("Starting download of %q.", partialPath)
+
+	err = downloadIcon(ctx, name, downloadURL, w)
+	if err != nil {
+		logger.Debugf("download of %q failed: %#v", downloadURL, err)
+		return err
+	}
+
+	if err = w.Sync(); err != nil {
+		return err
+	}
+
+	if err = w.Close(); err != nil {
+		return err
+	}
+
+	// Only rename once we know all is well, so we don't want to have any side
+	// effects on any existing snap icon if an error occurs.
+	return os.Rename(w.Name(), targetPath)
+}
+
 func downloadReqOpts(storeURL *url.URL, cdnHeader string, opts *DownloadOptions) *requestOptions {
 	reqOptions := requestOptions{
 		Method:       "GET",
@@ -600,6 +659,88 @@
 	return finalErr
 }
 
+var downloadIcon = downloadIconImpl
+
+// downloadIconImpl writes an http.Request which does not require authentication
+// or a progress.Meter.
+func downloadIconImpl(ctx context.Context, name, downloadURL string, w io.ReadWriteSeeker) error {
+	iconURL, err := url.Parse(downloadURL)
+	if err != nil {
+		return err
+	}
+
+	var finalErr error
+	startTime := time.Now()
+	for attempt := retry.Start(downloadRetryStrategy, nil); attempt.Next(); {
+		httputil.MaybeLogRetryAttempt(iconURL.String(), attempt, startTime)
+
+		if cancelled(ctx) {
+			return fmt.Errorf("the download has been cancelled: %s", ctx.Err())
+		}
+
+		clientOpts := &httputil.ClientOptions{} // don't need any options
+		cli := httputil.NewHTTPClient(clientOpts)
+		var resp *http.Response
+		resp, finalErr = doIconRequest(ctx, cli, iconURL)
+		if cancelled(ctx) {
+			return fmt.Errorf("the download has been cancelled: %s", ctx.Err())
+		}
+		if finalErr != nil {
+			if httputil.ShouldRetryAttempt(attempt, finalErr) {
+				continue
+			}
+			break
+		}
+
+		if httputil.ShouldRetryHttpResponse(attempt, resp) {
+			resp.Body.Close()
+			continue
+		}
+
+		// XXX: we're inside retry loop, so this will be closed only on return.
+		defer resp.Body.Close()
+
+		switch resp.StatusCode {
+		case 200: // OK
+		default:
+			return &DownloadError{Code: resp.StatusCode, URL: resp.Request.URL}
+		}
+
+		const maxIconFilesize int64 = 300000
+
+		if resp.ContentLength == 0 || resp.ContentLength > maxIconFilesize {
+			return fmt.Errorf("unsupported Content-Length for %s (must be nonzero and <%dB): %d", downloadURL, maxIconFilesize, resp.ContentLength)
+		} else {
+			logger.Debugf("Download size for %s: %d", downloadURL, resp.ContentLength)
+		}
+
+		var bytesWritten int64
+		bytesWritten, finalErr = io.Copy(w, resp.Body)
+
+		if cancelled(ctx) {
+			return fmt.Errorf("the download has been cancelled: %s", ctx.Err())
+		}
+
+		if finalErr != nil {
+			if httputil.ShouldRetryAttempt(attempt, finalErr) {
+				// XXX: is this correct, without resume? Or should we just
+				// error here since we're not doing Range requests?
+				// And should we seek w back to 0?
+				continue
+			}
+			break
+		}
+		if bytesWritten > maxIconFilesize {
+			return fmt.Errorf("the download size exceeded the max icon filesize (%dB), despite that the reported Content-Length was %d: %d", maxIconFilesize, resp.ContentLength, bytesWritten)
+		}
+		break
+	}
+	if finalErr == nil {
+		logger.Debugf("Icon download succeeded")
+	}
+	return finalErr
+}
+
 // DownloadStream will copy the snap from the request to the io.Reader
 func (s *Store) DownloadStream(ctx context.Context, name string, downloadInfo *snap.DownloadInfo, resume int64, user *auth.UserState) (io.ReadCloser, int, error) {
 	// most other store network operations use s.endpointURL, which returns an