#614 serve proxified images disregarding if saving is enabled

README.md

@@ -879,5 +879,6 @@ _all admin calls require auth and admin privilege_
 * User can edit comments in 5 mins (configurable) window after creation.
 * User ID hashed and prefixed by oauth provider name to avoid collisions and potential abuse.
 * All avatars resized and cached locally to prevent rate limiters from oauth providers, part of [go-pkgz/auth](https://github.com/go-pkgz/auth) functionality.
-* Images can be proxied (`IMG_PROXY=true`) to prevent mixed http/https.
+* Images can be proxied (`IMAGE_PROXY_HTTP2HTTPS=true`) to prevent mixed http/https.
+* All images can be proxied and saved (`IMAGE_PROXY_CACHE_EXTERNAL=true`) instead of serving from original location. Beware, images which are posted with this parameter enabled will be served from proxy even after it will be disabled.
 * Docker build uses [publicly available](https://github.com/umputun/baseimage) base images.

backend/app/rest/proxy/image.go

@@ -84,15 +84,6 @@ func (p Image) replace(commentHTML string, imgs []string) string {
 
 // Handler returns http handler respond to proxied request
 func (p Image) Handler(w http.ResponseWriter, r *http.Request) {
-	if !p.HTTP2HTTPS && !p.CacheExternal {
-		// TODO: we might need to find a better way to handle it. If admin enables caching/proxy and disables it later on
-		// all comments that got converted will lose their images. We can't just return a redirect (it will open an ability
-		// to redirect anywhere). We can probably continue proxying these images (but need to make sure this behavior is
-		// documented) or, better, provide a way to migrate back converted comments.
-		http.Error(w, "none of the proxy features are enabled", http.StatusNotImplemented)
-		return
-	}
-
 	src, err := base64.URLEncoding.DecodeString(r.URL.Query().Get("src"))
 	if err != nil {
 		rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't decode image url", rest.ErrDecode)
@@ -106,9 +97,8 @@ func (p Image) Handler(w http.ResponseWriter, r *http.Request) {
 		rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't parse image url "+imgURL, rest.ErrAssetNotFound)
 		return
 	}
-	if p.CacheExternal {
-		img, _ = p.ImageService.Load(imgID)
-	}
+	// try to load from cache for case it was saved when CacheExternal was enabled
+	img, _ = p.ImageService.Load(imgID)
 	if img == nil {
 		img, err = p.downloadImage(context.Background(), imgURL)
 		if err != nil {

backend/app/rest/proxy/image_test.go

@@ -96,7 +96,13 @@ func TestImage_Replace(t *testing.T) {
 }
 
 func TestImage_Routes(t *testing.T) {
-	img := Image{HTTP2HTTPS: true, RemarkURL: "https://demo.remark42.com", RoutePath: "/api/v1/proxy"}
+	imageStore := image.MockStore{}
+	img := Image{
+		HTTP2HTTPS:   true,
+		RemarkURL:    "https://demo.remark42.com",
+		RoutePath:    "/api/v1/proxy",
+		ImageService: image.NewService(&imageStore, image.ServiceParams{}),
+	}
 
 	ts := httptest.NewServer(http.HandlerFunc(img.Handler))
 	defer ts.Close()
@@ -105,21 +111,50 @@ func TestImage_Routes(t *testing.T) {
 
 	encodedImgURL := base64.URLEncoding.EncodeToString([]byte(httpSrv.URL + "/image/img1.png"))
 
+	// no image supposed to be cached
+	imageStore.On("Load", mock.Anything).Times(2).Return(nil, nil)
+
 	resp, err := http.Get(ts.URL + "/?src=" + encodedImgURL)
 	require.NoError(t, err)
-	assert.Equal(t, 200, resp.StatusCode)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
 	assert.Equal(t, "1462", resp.Header["Content-Length"][0])
 	assert.Equal(t, "image/png", resp.Header["Content-Type"][0])
 
 	encodedImgURL = base64.URLEncoding.EncodeToString([]byte(httpSrv.URL + "/image/no-such-image.png"))
 	resp, err = http.Get(ts.URL + "/?src=" + encodedImgURL)
 	require.NoError(t, err)
-	assert.Equal(t, 404, resp.StatusCode)
+	assert.Equal(t, http.StatusNotFound, resp.StatusCode)
 
 	encodedImgURL = base64.URLEncoding.EncodeToString([]byte(httpSrv.URL + "bad encoding"))
 	resp, err = http.Get(ts.URL + "/?src=" + encodedImgURL)
 	require.NoError(t, err)
-	assert.Equal(t, 400, resp.StatusCode)
+	assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
+}
+
+func TestImage_DisabledCachingAndHTTP2HTTPS(t *testing.T) {
+	imageStore := image.MockStore{}
+	img := Image{
+		RemarkURL:    "https://demo.remark42.com",
+		RoutePath:    "/api/v1/proxy",
+		ImageService: image.NewService(&imageStore, image.ServiceParams{}),
+	}
+
+	ts := httptest.NewServer(http.HandlerFunc(img.Handler))
+	defer ts.Close()
+	httpSrv := imgHTTPTestsServer(t)
+	defer httpSrv.Close()
+
+	encodedImgURL := base64.URLEncoding.EncodeToString([]byte(httpSrv.URL + "/image/img1.png"))
+
+	imageStore.On("Load", mock.Anything).Once().Return(nil, nil)
+
+	resp, err := http.Get(ts.URL + "/?src=" + encodedImgURL)
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+	assert.Equal(t, "1462", resp.Header["Content-Length"][0])
+	assert.Equal(t, "image/png", resp.Header["Content-Type"][0])
+
+	imageStore.AssertCalled(t, "Load", mock.Anything)
 }
 
 func TestImage_RoutesCachingImage(t *testing.T) {
@@ -145,7 +180,7 @@ func TestImage_RoutesCachingImage(t *testing.T) {
 
 	resp, err := http.Get(ts.URL + "/?src=" + encodedImgURL)
 	require.Nil(t, err)
-	assert.Equal(t, 200, resp.StatusCode)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
 	assert.Equal(t, "1462", resp.Header["Content-Length"][0])
 	assert.Equal(t, "image/png", resp.Header["Content-Type"][0])
 
@@ -176,7 +211,7 @@ func TestImage_RoutesUsingCachedImage(t *testing.T) {
 
 	resp, err := http.Get(ts.URL + "/?src=" + encodedImgURL)
 	require.Nil(t, err)
-	assert.Equal(t, 200, resp.StatusCode)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
 	assert.Equal(t, "256", resp.Header["Content-Length"][0])
 	assert.Equal(t, "text/plain; charset=utf-8", resp.Header["Content-Type"][0],
 		"if you save text you receive text/plain in response, that's only fair option you got")
@@ -185,17 +220,28 @@ func TestImage_RoutesUsingCachedImage(t *testing.T) {
 }
 
 func TestImage_RoutesTimedOut(t *testing.T) {
-	img := Image{HTTP2HTTPS: true, RemarkURL: "https://demo.remark42.com", RoutePath: "/api/v1/proxy", Timeout: 50 * time.Millisecond}
+	imageStore := image.MockStore{}
+	img := Image{
+		HTTP2HTTPS:   true,
+		RemarkURL:    "https://demo.remark42.com",
+		RoutePath:    "/api/v1/proxy",
+		Timeout:      50 * time.Millisecond,
+		ImageService: image.NewService(&imageStore, image.ServiceParams{}),
+	}
 
 	ts := httptest.NewServer(http.HandlerFunc(img.Handler))
 	defer ts.Close()
 	httpSrv := imgHTTPTestsServer(t)
 	defer httpSrv.Close()
 
 	encodedImgURL := base64.URLEncoding.EncodeToString([]byte(httpSrv.URL + "/image/img-slow.png"))
+
+	// no image supposed to be cached
+	imageStore.On("Load", mock.Anything).Once().Return(nil, nil)
+
 	resp, err := http.Get(ts.URL + "/?src=" + encodedImgURL)
 	require.NoError(t, err)
-	assert.Equal(t, 404, resp.StatusCode)
+	assert.Equal(t, http.StatusNotFound, resp.StatusCode)
 	b, err := ioutil.ReadAll(resp.Body)
 	require.NoError(t, err)
 	t.Log(string(b))