From fe716b0a717a732168e2e78fb97fa98fd3cb3a21 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Thu, 13 May 2021 03:05:56 +0200 Subject: [PATCH] validate image existence before post or preview --- backend/app/rest/api/rest_private.go | 14 ++++++++++++ backend/app/rest/api/rest_private_test.go | 26 ++++++++++++++++++++++- backend/app/rest/api/rest_public.go | 15 +++++++++++++ backend/app/rest/api/rest_public_test.go | 21 ++++++++++++++++++ backend/app/rest/api/rest_test.go | 6 +++++- backend/app/rest/httperrors.go | 1 + backend/app/store/image/image.go | 2 ++ 7 files changed, 83 insertions(+), 2 deletions(-) diff --git a/backend/app/rest/api/rest_private.go b/backend/app/rest/api/rest_private.go index 3caad19a..3db49cbb 100644 --- a/backend/app/rest/api/rest_private.go +++ b/backend/app/rest/api/rest_private.go @@ -88,6 +88,20 @@ func (s *private) createCommentCtrl(w http.ResponseWriter, r *http.Request) { } comment = s.commentFormatter.Format(comment) + // check if images are valid + imgIds, err := s.imageService.ExtractPictures(comment.Text) + if err != nil { + rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't extract pictures from comment text", rest.ErrCommentValidation) + return + } + for _, id := range imgIds { + _, err = s.imageService.Load(id) + if err != nil { + rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't load picture from the comment", rest.ErrImgNotFound) + return + } + } + // check if user blocked if s.dataService.IsBlocked(comment.Locator.SiteID, comment.User.ID) { rest.SendErrorJSON(w, r, http.StatusForbidden, errors.New("rejected"), "user blocked", rest.ErrUserBlocked) diff --git a/backend/app/rest/api/rest_private_test.go b/backend/app/rest/api/rest_private_test.go index 12c5ec5f..c3382156 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -165,6 +165,28 @@ func TestRest_CreateRejected(t *testing.T) { require.Equal(t, http.StatusForbidden, resp.StatusCode, "reject wrong aud") } +func TestRest_CreateWithWrongImage(t *testing.T) { + ts, srv, teardown := startupT(t) + defer teardown() + + // create comment + resp, err := post(t, ts.URL+"/api/v1/comment", fmt.Sprintf(`{"text": "![non-existent.jpg](%s/api/v1/picture/dev_user/bad_picture)", "locator":{"url": "https://radio-t.com/blah1", "site": "radio-t"}}`, srv.RemarkURL)) + assert.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + b, err := ioutil.ReadAll(resp.Body) + assert.NoError(t, err) + assert.NoError(t, resp.Body.Close()) + assert.Contains(t, + string(b), + "{\"code\":20,\"details\":\"can't load picture from the comment\","+ + "\"error\":\"can't get image file for dev_user/bad_picture: can't get image stats for dev_user/bad_picture: stat ", + ) + assert.Contains(t, + string(b), + "/pics-remark42/staging/dev_user/62/bad_picture: no such file or directory\"}\n", + ) +} + func TestRest_CreateWithLazyImage(t *testing.T) { ts, _, teardown := startupT(t) defer teardown() @@ -979,6 +1001,8 @@ func TestRest_CreateWithPictures(t *testing.T) { }, image.ServiceParams{ EditDuration: 100 * time.Millisecond, MaxSize: 2000, + ImageAPI: svc.RemarkURL + "/api/v1/picture/", + ProxyAPI: svc.RemarkURL + "/api/v1/img", }) defer imageService.Close(context.Background()) @@ -1022,7 +1046,7 @@ func TestRest_CreateWithPictures(t *testing.T) { ids[i] = uploadPicture(fmt.Sprintf("pic%d.png", i)) } - text := fmt.Sprintf(`text 123 ![](/api/v1/picture/%s) *xxx* ![](/api/v1/picture/%s) ![](/api/v1/picture/%s)`, ids[0], ids[1], ids[2]) + text := fmt.Sprintf(`text 123 ![](%s/api/v1/picture/%s) *xxx* ![](%s/api/v1/picture/%s) ![](%s/api/v1/picture/%s)`, svc.RemarkURL, ids[0], svc.RemarkURL, ids[1], svc.RemarkURL, ids[2]) body := fmt.Sprintf(`{"text": "%s", "locator":{"url": "https://radio-t.com/blah1", "site": "remark42"}}`, text) resp, err := post(t, ts.URL+"/api/v1/comment", body) diff --git a/backend/app/rest/api/rest_public.go b/backend/app/rest/api/rest_public.go index 93711374..f6c7efb0 100644 --- a/backend/app/rest/api/rest_public.go +++ b/backend/app/rest/api/rest_public.go @@ -132,6 +132,21 @@ func (s *public) previewCommentCtrl(w http.ResponseWriter, r *http.Request) { comment = s.commentFormatter.Format(comment) comment.Sanitize() + + // check if images are valid + imgIds, err := s.imageService.ExtractPictures(comment.Text) + if err != nil { + rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't extract pictures from comment text", rest.ErrCommentValidation) + return + } + for _, id := range imgIds { + _, err = s.imageService.Load(id) + if err != nil { + rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't load picture from the comment", rest.ErrImgNotFound) + return + } + } + render.HTML(w, r, comment.Text) } diff --git a/backend/app/rest/api/rest_public_test.go b/backend/app/rest/api/rest_public_test.go index 8c1e987b..39326c12 100644 --- a/backend/app/rest/api/rest_public_test.go +++ b/backend/app/rest/api/rest_public_test.go @@ -45,6 +45,27 @@ func TestRest_Preview(t *testing.T) { assert.Equal(t, 400, resp.StatusCode) } +func TestRest_PreviewWithWrongImage(t *testing.T) { + ts, srv, teardown := startupT(t) + defer teardown() + + resp, err := post(t, ts.URL+"/api/v1/preview", fmt.Sprintf(`{"text": "![non-existent.jpg](%s/api/v1/picture/dev_user/bad_picture)", "locator":{"url": "https://radio-t.com/blah1", "site": "radio-t"}}`, srv.RemarkURL)) + assert.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + b, err := ioutil.ReadAll(resp.Body) + assert.NoError(t, err) + assert.NoError(t, resp.Body.Close()) + assert.Contains(t, + string(b), + "{\"code\":20,\"details\":\"can't load picture from the comment\","+ + "\"error\":\"can't get image file for dev_user/bad_picture: can't get image stats for dev_user/bad_picture: stat ", + ) + assert.Contains(t, + string(b), + "/pics-remark42/staging/dev_user/62/bad_picture: no such file or directory\"}\n", + ) +} + func TestRest_PreviewWithMD(t *testing.T) { ts, _, teardown := startupT(t) defer teardown() diff --git a/backend/app/rest/api/rest_test.go b/backend/app/rest/api/rest_test.go index 240e340a..6effe33d 100644 --- a/backend/app/rest/api/rest_test.go +++ b/backend/app/rest/api/rest_test.go @@ -402,6 +402,8 @@ func startupT(t *testing.T) (ts *httptest.Server, srv *Rest, teardown func()) { RestrictedWordsMatcher: restrictedWordsMatcher, } + remarkURL := "https://demo.remark42.com" + srv = &Rest{ DataService: dataStore, Authenticator: auth.NewService(auth.Opts{ @@ -411,13 +413,15 @@ func startupT(t *testing.T) (ts *httptest.Server, srv *Rest, teardown func()) { }), Cache: memCache, WebRoot: tmp, - RemarkURL: "https://demo.remark42.com", + RemarkURL: remarkURL, ImageService: image.NewService(&image.FileSystem{ Location: tmp + "/pics-remark42", Partitions: 100, Staging: tmp + "/pics-remark42/staging", }, image.ServiceParams{ EditDuration: 100 * time.Millisecond, + ImageAPI: remarkURL + "/api/v1/picture/", + ProxyAPI: remarkURL + "/api/v1/img", MaxSize: 10000, }), ImageProxy: &proxy.Image{}, diff --git a/backend/app/rest/httperrors.go b/backend/app/rest/httperrors.go index 3a094b8e..d5310590 100644 --- a/backend/app/rest/httperrors.go +++ b/backend/app/rest/httperrors.go @@ -39,6 +39,7 @@ const ( ErrActionRejected = 17 // general error for rejected actions ErrAssetNotFound = 18 // requested file not found ErrCommentRestrictWords = 19 // restricted words in a comment + ErrImgNotFound = 20 // posted image not found in the storage ) // errTmplData store data for error message diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index c346d490..e61ce248 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -148,6 +148,8 @@ 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 +// It doesn't return possible errors parsing the cached images, and will try to return as many parsed ids +// as possible instead. func (s *Service) ExtractPictures(commentHTML string) (ids []string, err error) { doc, err := goquery.NewDocumentFromReader(strings.NewReader(commentHTML))