Skip to content

Commit

Permalink
remove proxied images from sanity check
Browse files Browse the repository at this point in the history
Previously, proxied and local images were checked for presence in the
storage before previewing or posting the comment. That logic resulted in
 an inability to post with an image when a proxy for images is enabled,
 as proxied images are not downloaded to disk before the first time
 someone loads them, which could only happen after the user either
 previews or posts the message.

After this change, preview and post only checks the local images'
presence and ignore the proxied ones.
  • Loading branch information
paskal committed Jul 20, 2023
1 parent 5aa4bf2 commit 0258b15
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 13 deletions.
8 changes: 4 additions & 4 deletions backend/app/rest/api/rest_private.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ 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.ExtractPictures(comment.Text, false) {
err := s.imageService.ResetCleanupTimer(id)
if err != nil {
rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't load picture from the comment", rest.ErrImgNotFound)
Expand Down Expand Up @@ -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.ExtractPictures(comment.Text, false) {
_, err := s.imageService.Load(id)
if err != nil {
rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't load picture from the comment", rest.ErrImgNotFound)
Expand Down
7 changes: 5 additions & 2 deletions backend/app/store/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,10 @@ 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) {
// Proxied images are returned only if the flag is set: this 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) ExtractPictures(commentHTML string, returnProxied 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)
Expand All @@ -155,7 +158,7 @@ func (s *Service) ExtractPictures(commentHTML string) (ids []string) {
ids = append(ids, id)
}
}
if strings.Contains(im, s.ProxyAPI) {
if strings.Contains(im, s.ProxyAPI) && returnProxied {
proxiedURL, err := url.Parse(im)
if err != nil {
return
Expand Down
15 changes: 9 additions & 6 deletions backend/app/store/image/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestService_ExtractPictures(t *testing.T) {
svc := Service{ServiceParams: ServiceParams{ImageAPI: "/blah/", ProxyAPI: "/non_existent"}}
html := `blah <img src="/blah/user1/pic1.png"/> foo
<img src="/blah/user2/pic3.png"/> xyz <p>123</p> <img src="/pic3.png"/> <img src="https://i.ibb.co/0cqqqnD/ezgif-5-3b07b6b97610.png" alt="">`
ids := svc.ExtractPictures(html)
ids := svc.ExtractPictures(html, false)
require.Equal(t, 2, len(ids), "two images")
assert.Equal(t, "user1/pic1.png", ids[0])
assert.Equal(t, "user2/pic3.png", ids[1])
Expand All @@ -96,30 +96,33 @@ func TestService_ExtractPictures(t *testing.T) {
<p><img src="https://remark42.radio-t.com/api/v1/picture/github_ef0f706a79cc24b17bbbb374cd234a691d034128/bjttt8ahajfmrhsula10.png" alt="bjtr0-201906-08110846-i324c.png"/></p>\n\n<p>
По форме все верно, это все packages, но по сути это все одна библиотека организованная таким образом. При ее импорте, например посредством go mod, она выглядит как один модуль, т.е.
<code>github.com/go-pkgz/auth v0.5.2</code>.</p>\n`
ids = svc.ExtractPictures(html)
ids = svc.ExtractPictures(html, false)
require.Equal(t, 1, len(ids), "one image in")
assert.Equal(t, "github_ef0f706a79cc24b17bbbb374cd234a691d034128/bjttt8ahajfmrhsula10.png", ids[0])

// proxied image
html = `<img src="https://remark42.radio-t.com/api/v1/img?src=aHR0cHM6Ly9ob21lcGFnZXMuY2FlLndpc2MuZWR1L35lY2U1MzMvaW1hZ2VzL2JvYXQucG5n" alt="cat.png">`
ids = svc.ExtractPictures(html)
ids = svc.ExtractPictures(html, true)
require.Equal(t, 1, len(ids), "one image in")
assert.Equal(t, "cached_images/12318fbd4c55e9d177b8b5ae197bc89c5afd8e07-a41fcb00643f28d700504256ec81cbf2e1aac53e", ids[0])
// same proxied image, with returnProxied set to false
ids = svc.ExtractPictures(html, false)
require.Empty(t, ids, "no images expected to be found")

// bad url
html = `<img src=" https://remark42.radio-t.com/api/v1/img">`
ids = svc.ExtractPictures(html)
ids = svc.ExtractPictures(html, false)
require.Empty(t, ids)

// bad src
html = `<img src="https://remark42.radio-t.com/api/v1/img?src=bad">`
ids = svc.ExtractPictures(html)
ids = svc.ExtractPictures(html, false)
require.Empty(t, ids)

// good src with bad content
badURL := base64.URLEncoding.EncodeToString([]byte(" http://foo.bar"))
html = fmt.Sprintf(`<img src="https://remark42.radio-t.com/api/v1/img?src=%s">`, badURL)
ids = svc.ExtractPictures(html)
ids = svc.ExtractPictures(html, false)
require.Empty(t, ids)
}

Expand Down
2 changes: 1 addition & 1 deletion backend/app/store/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ func (s *DataStore) submitImages(comment store.Comment) {
log.Printf("[WARN] can't get comment's %s text for image extraction, %v", comment.ID, err)
return nil
}
imgIDs := s.ImageService.ExtractPictures(cc.Text)
imgIDs := s.ImageService.ExtractPictures(cc.Text, true)
if len(imgIDs) > 0 {
log.Printf("[DEBUG] image ids extracted from %s - %+v", comment.ID, imgIDs)
}
Expand Down

0 comments on commit 0258b15

Please sign in to comment.