Skip to content

Commit

Permalink
validate image existence before post or preview
Browse files Browse the repository at this point in the history
  • Loading branch information
paskal committed May 15, 2021
1 parent d1ef3ff commit f00d433
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 11 deletions.
14 changes: 14 additions & 0 deletions backend/app/rest/api/rest_private.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 25 additions & 1 deletion backend/app/rest/api/rest_private_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions backend/app/rest/api/rest_public.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
21 changes: 21 additions & 0 deletions backend/app/rest/api/rest_public_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 5 additions & 1 deletion backend/app/rest/api/rest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{},
Expand Down
1 change: 1 addition & 0 deletions backend/app/rest/httperrors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 12 additions & 6 deletions backend/app/store/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,22 +164,28 @@ func (s *Service) ExtractPictures(commentHTML string) (ids []string, err error)
}
}
if strings.Contains(im, s.ProxyAPI) {
proxiedURL, err := url.Parse(im)
if err != nil {
proxiedURL, e := url.Parse(im)
if e != nil {
err = errors.Wrapf(e, "problem parsing %s", im)
return
}
imgURL, err := base64.URLEncoding.DecodeString(proxiedURL.Query().Get("src"))
if err != nil {
imgURL, e := base64.URLEncoding.DecodeString(proxiedURL.Query().Get("src"))
if e != nil {
err = errors.Wrapf(e, "problem decodind base64 from %s", im)
return
}
imgID, err := CachedImgID(string(imgURL))
if err != nil {
imgID, e := CachedImgID(string(imgURL))
if e != nil {
err = errors.Wrapf(e, "problem retrieving cached URL for %s", im)
return
}
ids = append(ids, imgID)
}
}
})
if err != nil {
return nil, err
}

return ids, nil
}
Expand Down
6 changes: 3 additions & 3 deletions backend/app/store/image/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,20 +105,20 @@ func TestService_ExtractPictures(t *testing.T) {
// bad url
html = `<img src=" https://remark42.radio-t.com/api/v1/img">`
ids, err = svc.ExtractPictures(html)
require.NoError(t, err)
require.Error(t, err)
require.Empty(t, ids)

// bad src
html = `<img src="https://remark42.radio-t.com/api/v1/img?src=bad">`
ids, err = svc.ExtractPictures(html)
require.NoError(t, err)
require.Error(t, err)
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, err = svc.ExtractPictures(html)
require.NoError(t, err)
require.Error(t, err)
require.Empty(t, ids)
}

Expand Down

0 comments on commit f00d433

Please sign in to comment.