Fix preview and comment work with proxified images

backend/app/rest/api/rest_private.go

@@ -90,11 +90,11 @@ func (s *private) previewCommentCtrl(w http.ResponseWriter, r *http.Request) {
 	comment = s.commentFormatter.Format(comment)
 	comment.Sanitize()
 
-	// check if images are valid
-	for _, id := range s.imageService.ExtractPictures(comment.Text) {
+	// check if images are valid, omit proxied images as they are lazy-loaded
+	for _, id := range s.imageService.ExtractNonProxiedPictures(comment.Text) {
 		err := s.imageService.ResetCleanupTimer(id)
 		if err != nil {
-			rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't renew staged picture cleanup timer", rest.ErrImgNotFound)
+			rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't load picture from the comment", rest.ErrImgNotFound)
 			return
 		}
 	}
@@ -129,8 +129,8 @@ func (s *private) createCommentCtrl(w http.ResponseWriter, r *http.Request) {
 	}
 	comment = s.commentFormatter.Format(comment)
 
-	// check if images are valid
-	for _, id := range s.imageService.ExtractPictures(comment.Text) {
+	// check if images are valid, omit proxied images as they are lazy-loaded
+	for _, id := range s.imageService.ExtractNonProxiedPictures(comment.Text) {
 		_, err := s.imageService.Load(id)
 		if err != nil {
 			rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't load picture from the comment", rest.ErrImgNotFound)

backend/app/rest/api/rest_private_test.go

@@ -10,6 +10,7 @@ import (
 	"io"
 	"mime/multipart"
 	"net/http"
+	"net/http/httptest"
 	"os"
 	"strings"
 	"testing"
@@ -24,6 +25,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/umputun/remark42/backend/app/notify"
+	"github.com/umputun/remark42/backend/app/rest/proxy"
 	"github.com/umputun/remark42/backend/app/store"
 	"github.com/umputun/remark42/backend/app/store/image"
 )
@@ -69,7 +71,7 @@ func TestRest_CreateFilteredCode(t *testing.T) {
 
 	c := R.JSON{}
 	err = json.Unmarshal(b, &c)
-	assert.NoError(t, err)
+	require.NoError(t, err, string(b))
 	loc := c["locator"].(map[string]interface{})
 	assert.Equal(t, "remark42", loc["site"])
 	assert.Equal(t, "https://radio-t.com/blah1", loc["url"])
@@ -79,6 +81,86 @@ func TestRest_CreateFilteredCode(t *testing.T) {
 	assert.True(t, len(c["id"].(string)) > 8)
 }
 
+// based on issue https://github.com/umputun/remark42/issues/1631
+func TestRest_CreateAndPreviewWithImage(t *testing.T) {
+	ts, srv, teardown := startupT(t)
+	ts.Close()
+	defer teardown()
+
+	srv.ImageService.ProxyAPI = srv.RemarkURL + "/api/v1/img"
+	srv.ImageProxy = &proxy.Image{
+		HTTP2HTTPS:    true,
+		CacheExternal: true,
+		RoutePath:     "/api/v1/img",
+		RemarkURL:     srv.RemarkURL,
+		ImageService:  srv.ImageService,
+	}
+	srv.CommentFormatter = store.NewCommentFormatter(srv.ImageProxy)
+	// need to recreate the server with new ImageProxy, otherwise old one will be used
+	ts = httptest.NewServer(srv.routes())
+	defer ts.Close()
+
+	var pngRead bool
+	// server with the test PNG image
+	pngServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, e := io.Copy(w, gopherPNG())
+		assert.NoError(t, e)
+		pngRead = true
+	}))
+	defer pngServer.Close()
+
+	t.Run("create", func(t *testing.T) {
+		resp, err := post(t, ts.URL+"/api/v1/comment",
+			`{"text": "![](`+pngServer.URL+`/gopher.png)", "locator":{"url": "https://radio-t.com/blah1", "site": "remark42"}}`)
+		assert.NoError(t, err)
+		b, err := io.ReadAll(resp.Body)
+		assert.NoError(t, err)
+		require.Equal(t, http.StatusCreated, resp.StatusCode, string(b))
+		assert.NoError(t, resp.Body.Close())
+
+		c := R.JSON{}
+		err = json.Unmarshal(b, &c)
+		require.NoError(t, err, string(b))
+		assert.NotContains(t, c["text"], pngServer.URL)
+		assert.Contains(t, c["text"], srv.RemarkURL)
+		loc := c["locator"].(map[string]interface{})
+		assert.Equal(t, "remark42", loc["site"])
+		assert.Equal(t, "https://radio-t.com/blah1", loc["url"])
+		assert.True(t, len(c["id"].(string)) > 8)
+		assert.Equal(t, false, pngRead, "original image is not yet accessed by server")
+	})
+
+	t.Run("preview", func(t *testing.T) {
+		resp, err := post(t, ts.URL+"/api/v1/preview",
+			`{"text": "![](`+pngServer.URL+`/gopher.png)", "locator":{"url": "https://radio-t.com/blah1", "site": "remark42"}}`)
+		assert.NoError(t, err)
+		b, err := io.ReadAll(resp.Body)
+		assert.NoError(t, err)
+		require.Equal(t, http.StatusOK, resp.StatusCode, string(b))
+		assert.NoError(t, resp.Body.Close())
+
+		assert.NotContains(t, string(b), pngServer.URL)
+		assert.Contains(t, string(b), srv.RemarkURL)
+
+		assert.Equal(t, false, pngRead, "original image is not yet accessed by server")
+		// retrieve the image from the cache
+		imgURL := strings.Split(strings.Split(string(b), "src=\"")[1], "\"")[0]
+		// replace srv.RemarkURL with ts.URL
+		imgURL = strings.ReplaceAll(imgURL, srv.RemarkURL, ts.URL)
+		resp, err = http.Get(imgURL)
+		assert.NoError(t, err)
+		b, err = io.ReadAll(resp.Body)
+		assert.NoError(t, err)
+		require.Equal(t, http.StatusOK, resp.StatusCode, string(b))
+		assert.NoError(t, resp.Body.Close())
+		// compare image to original gopher png after decoding from base64
+		assert.Equal(t, gopher, base64.StdEncoding.EncodeToString(b))
+
+		assert.Equal(t, true, pngRead, "original image accessed to be shown to user")
+	})
+
+}
+
 func TestRest_CreateOldPost(t *testing.T) {
 	ts, srv, teardown := startupT(t)
 	defer teardown()

backend/app/rest/api/rest_public_test.go

@@ -76,8 +76,8 @@ func TestRest_Preview(t *testing.T) {
 	assert.NoError(t, resp.Body.Close())
 	assert.Contains(t,
 		string(b),
-		"{\"code\":20,\"details\":\"can't renew staged picture cleanup timer\","+
-			"\"error\":\"can't get image stats for dev_user/bad_picture: stat",
+		`{"code":20,"details":"can't load picture from the comment",`+
+			`"error":"can't get image stats for dev_user/bad_picture: stat`,
 	)
 	assert.Contains(t,
 		string(b),
@@ -97,8 +97,8 @@ func TestRest_PreviewWithWrongImage(t *testing.T) {
 	assert.NoError(t, resp.Body.Close())
 	assert.Contains(t,
 		string(b),
-		"{\"code\":20,\"details\":\"can't renew staged picture cleanup timer\","+
-			"\"error\":\"can't get image stats for dev_user/bad_picture: stat ",
+		`{"code":20,"details":"can't load picture from the comment",`+
+			`"error":"can't get image stats for dev_user/bad_picture: stat `,
 	)
 	assert.Contains(t,
 		string(b),

backend/app/store/image/image.go

@@ -88,8 +88,8 @@ func NewService(s Store, p ServiceParams) *Service {
 	return &Service{ServiceParams: p, store: s}
 }
 
-// SubmitAndCommit multiple ids immediately
-func (s *Service) SubmitAndCommit(idsFn func() []string) error {
+// Commit multiple ids immediately
+func (s *Service) Commit(idsFn func() []string) error {
 	errs := new(multierror.Error)
 	for _, id := range idsFn() {
 		err := s.store.Commit(id)
@@ -117,7 +117,7 @@ func (s *Service) Submit(idsFn func() []string) {
 				for atomic.LoadInt32(&s.term) == 0 && time.Since(req.TS) <= s.EditDuration {
 					time.Sleep(time.Millisecond * 10) // small sleep to relive busy wait but keep reactive for term (close)
 				}
-				err := s.SubmitAndCommit(req.idsFn)
+				err := s.Commit(req.idsFn)
 				if err != nil {
 					log.Printf("[WARN] image commit error %v", err)
 				}
@@ -141,39 +141,14 @@ func (s *Service) Submit(idsFn func() []string) {
 
 // ExtractPictures gets list of images from the doc html and convert from urls to ids, i.e. user/pic.png
 func (s *Service) ExtractPictures(commentHTML string) (ids []string) {
-	doc, err := goquery.NewDocumentFromReader(strings.NewReader(commentHTML))
-	if err != nil {
-		log.Printf("[ERROR] can't parse commentHTML to parse images: %q, error: %v", commentHTML, err)
-		return nil
-	}
-	doc.Find("img").Each(func(i int, sl *goquery.Selection) {
-		if im, ok := sl.Attr("src"); ok {
-			if strings.Contains(im, s.ImageAPI) {
-				elems := strings.Split(im, "/")
-				if len(elems) >= 2 {
-					id := elems[len(elems)-2] + "/" + elems[len(elems)-1]
-					ids = append(ids, id)
-				}
-			}
-			if strings.Contains(im, s.ProxyAPI) {
-				proxiedURL, err := url.Parse(im)
-				if err != nil {
-					return
-				}
-				imgURL, err := base64.URLEncoding.DecodeString(proxiedURL.Query().Get("src"))
-				if err != nil {
-					return
-				}
-				imgID, err := CachedImgID(string(imgURL))
-				if err != nil {
-					return
-				}
-				ids = append(ids, imgID)
-			}
-		}
-	})
+	return s.extractImageIDs(commentHTML, true)
+}
 
-	return ids
+// ExtractNonProxiedPictures gets list of non-proxied images from the doc html and convert from urls to ids, i.e. user/pic.png
+// This method is used in image check on post preview and load, as proxied images have lazy loading
+// and wouldn't be present on disk but still valid as they will be loaded the first time someone requests them.
+func (s *Service) ExtractNonProxiedPictures(commentHTML string) (ids []string) {
+	return s.extractImageIDs(commentHTML, false)
 }
 
 // Cleanup runs periodic cleanup with 1.5*ServiceParams.EditDuration. Blocking loop, should be called inside of goroutine by consumer
@@ -262,6 +237,43 @@ func (s *Service) ImgContentType(img []byte) string {
 	return contentType
 }
 
+// returns list of image IDs from the comment html, including proxied images if includeProxied is true
+func (s *Service) extractImageIDs(commentHTML string, includeProxied bool) (ids []string) {
+	doc, err := goquery.NewDocumentFromReader(strings.NewReader(commentHTML))
+	if err != nil {
+		log.Printf("[ERROR] can't parse commentHTML to parse images: %q, error: %v", commentHTML, err)
+		return nil
+	}
+	doc.Find("img").Each(func(i int, sl *goquery.Selection) {
+		if im, ok := sl.Attr("src"); ok {
+			if strings.Contains(im, s.ImageAPI) {
+				elems := strings.Split(im, "/")
+				if len(elems) >= 2 {
+					id := elems[len(elems)-2] + "/" + elems[len(elems)-1]
+					ids = append(ids, id)
+				}
+			}
+			if includeProxied && strings.Contains(im, s.ProxyAPI) {
+				proxiedURL, err := url.Parse(im)
+				if err != nil {
+					return
+				}
+				imgURL, err := base64.URLEncoding.DecodeString(proxiedURL.Query().Get("src"))
+				if err != nil {
+					return
+				}
+				imgID, err := CachedImgID(string(imgURL))
+				if err != nil {
+					return
+				}
+				ids = append(ids, imgID)
+			}
+		}
+	})
+
+	return ids
+}
+
 // prepareImage calls readAndValidateImage and resize on provided image.
 func (s *Service) prepareImage(r io.Reader) ([]byte, error) {
 	data, err := readAndValidateImage(r, s.MaxSize)

backend/app/store/image/image_test.go

@@ -105,6 +105,7 @@ func TestService_ExtractPictures(t *testing.T) {
 	ids = svc.ExtractPictures(html)
 	require.Equal(t, 1, len(ids), "one image in")
 	assert.Equal(t, "cached_images/12318fbd4c55e9d177b8b5ae197bc89c5afd8e07-a41fcb00643f28d700504256ec81cbf2e1aac53e", ids[0])
+	require.Empty(t, svc.ExtractNonProxiedPictures(html), "no non-proxied images expected to be found")
 
 	// bad url
 	html = `<img src=" https://remark42.radio-t.com/api/v1/img">`
@@ -150,7 +151,7 @@ func TestService_Submit(t *testing.T) {
 	svc := NewService(&store, ServiceParams{ImageAPI: "/blah/", EditDuration: time.Millisecond * 100})
 	svc.Submit(func() []string { return []string{"id1", "id2", "id3"} })
 	assert.Equal(t, 3, len(store.ResetCleanupTimerCalls()))
-	err := svc.SubmitAndCommit(func() []string { return []string{"id4", "id5"} })
+	err := svc.Commit(func() []string { return []string{"id4", "id5"} })
 	assert.NoError(t, err)
 	svc.Submit(func() []string { return []string{"id6", "id7"} })
 	assert.Equal(t, 5, len(store.ResetCleanupTimerCalls()))

backend/app/store/service/service.go

@@ -279,7 +279,7 @@ func (s *DataStore) submitImages(comment store.Comment) {
 
 	var err error
 	if comment.Imported {
-		err = s.ImageService.SubmitAndCommit(idsFn)
+		err = s.ImageService.Commit(idsFn)
 	} else {
 		s.ImageService.Submit(idsFn)
 	}