From 484e813dca7962000f648305a64ae8145d778a73 Mon Sep 17 00:00:00 2001
From: Claudio Costa <cstcld91@gmail.com>
Date: Mon, 20 Jul 2020 10:08:52 +0200
Subject: [PATCH] Return 400 error if limit_after is zero (#15049)

---
 api4/post.go                  |  5 +++++
 api4/post_test.go             |  8 +++++++-
 store/sqlstore/post_store.go  |  5 +++++
 store/storetest/post_store.go | 13 +++++++++++++
 4 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/api4/post.go b/api4/post.go
index 8f8ccc50e4..0f5a50dfa3 100644
--- a/api4/post.go
+++ b/api4/post.go
@@ -233,6 +233,11 @@ func getPostsForChannelAroundLastUnread(c *Context, w http.ResponseWriter, r *ht
 		return
 	}
 
+	if c.Params.LimitAfter == 0 {
+		c.SetInvalidUrlParam("limit_after")
+		return
+	}
+
 	skipFetchThreads := r.URL.Query().Get("skipFetchThreads") == "true"
 	postList, err := c.App.GetPostsForChannelAroundLastUnread(channelId, userId, c.Params.LimitBefore, c.Params.LimitAfter, skipFetchThreads)
 	if err != nil {
diff --git a/api4/post_test.go b/api4/post_test.go
index d6078e92c0..91f67e1686 100644
--- a/api4/post_test.go
+++ b/api4/post_test.go
@@ -1696,8 +1696,14 @@ func TestGetPostsForChannelAroundLastUnread(t *testing.T) {
 		require.Equal(t, namePost(expected.PrevPostId), namePost(actual.PrevPostId), "unexpected prev post id")
 	}
 
+	// Setting limit_after to zero should fail with a 400 BadRequest.
+	posts, resp := Client.GetPostsAroundLastUnread(userId, channelId, 20, 0)
+	require.Error(t, resp.Error)
+	require.Equal(t, "api.context.invalid_url_param.app_error", resp.Error.Id)
+	require.Equal(t, http.StatusBadRequest, resp.StatusCode)
+
 	// All returned posts are all read by the user, since it's created by the user itself.
-	posts, resp := Client.GetPostsAroundLastUnread(userId, channelId, 20, 20)
+	posts, resp = Client.GetPostsAroundLastUnread(userId, channelId, 20, 20)
 	CheckNoError(t, resp)
 	require.Len(t, posts.Order, 12, "Should return 12 posts only since there's no unread post")
 
diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go
index 6557f0ad00..51761c7222 100644
--- a/store/sqlstore/post_store.go
+++ b/store/sqlstore/post_store.go
@@ -691,6 +691,11 @@ func (s *SqlPostStore) GetPostsAfter(options model.GetPostsOptions) (*model.Post
 }
 
 func (s *SqlPostStore) getPostsAround(before bool, options model.GetPostsOptions) (*model.PostList, *model.AppError) {
+	if options.Page < 0 || options.PerPage < 0 {
+		return nil, model.NewAppError("SqlPostStore.GetPostContext", "store.sql_post.get_posts_around.get.app_error", nil,
+			fmt.Sprintf("Page=%d and PerPage=%d must be non-negative", options.Page, options.PerPage), http.StatusBadRequest)
+	}
+
 	offset := options.Page * options.PerPage
 	var posts, parents []*model.Post
 
diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go
index 4f56590f2b..e89aed3af0 100644
--- a/store/storetest/post_store.go
+++ b/store/storetest/post_store.go
@@ -5,6 +5,7 @@ package storetest
 
 import (
 	"fmt"
+	"net/http"
 	"sort"
 	"strings"
 	"testing"
@@ -893,6 +894,18 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 			time.Sleep(time.Millisecond)
 		}
 
+		t.Run("should return error if negative Page/PerPage options are passed", func(t *testing.T) {
+			postList, err := ss.Post().GetPostsAfter(model.GetPostsOptions{ChannelId: channelId, PostId: posts[0].Id, Page: 0, PerPage: -1})
+			assert.Nil(t, postList)
+			assert.Error(t, err)
+			assert.Equal(t, http.StatusBadRequest, err.StatusCode)
+
+			postList, err = ss.Post().GetPostsAfter(model.GetPostsOptions{ChannelId: channelId, PostId: posts[0].Id, Page: -1, PerPage: 10})
+			assert.Nil(t, postList)
+			assert.Error(t, err)
+			assert.Equal(t, http.StatusBadRequest, err.StatusCode)
+		})
+
 		t.Run("should not return anything before the first post", func(t *testing.T) {
 			postList, err := ss.Post().GetPostsBefore(model.GetPostsOptions{ChannelId: channelId, PostId: posts[0].Id, Page: 0, PerPage: 10})
 			assert.Nil(t, err)