diff --git a/README.md b/README.md index 11dceca..b84a68b 100644 --- a/README.md +++ b/README.md @@ -179,8 +179,10 @@ A message has: the SimpleX relay, to the second; for a sent one, when the bot sent it. -A `count` other than a whole number from 1 to 100 gets `400`, and an -`id` that `GET /api/v1/chats` does not list gets `404`. +The answer is `400` if the query cannot be read (it holds a `;`, or a +`%` not followed by two hexadecimal digits) or `count` is not a whole +number from 1 to 100, and `404` if `GET /api/v1/chats` does not list +`id`. ### `POST /api/v1/chats/{id}/messages` diff --git a/internal/api/messages.go b/internal/api/messages.go index 7b3ebe6..8c0fb34 100644 --- a/internal/api/messages.go +++ b/internal/api/messages.go @@ -70,7 +70,16 @@ func (h *handlers) handleMessages() http.HandlerFunc { return } - count, ok := messageCount(r) + // Not r.URL.Query, which drops a pair it cannot decode and so + // would turn count=1% into the default. + query, err := url.ParseQuery(r.URL.RawQuery) + if err != nil { + h.respondError(w, http.StatusBadRequest, "the query cannot be read") + + return + } + + count, ok := messageCount(query) if !ok { h.respondError(w, http.StatusBadRequest, "count must be a whole number from 1 to "+strconv.Itoa(maxCount)) @@ -181,16 +190,9 @@ func (h *handlers) respondChatError( } } -// messageCount returns the request's count, defaultCount if it has none, -// and false if it is anything but a whole number from 1 to maxCount. The -// query is parsed here because r.URL.Query drops a pair it cannot -// decode, which would turn count=1% into the default. -func messageCount(r *http.Request) (int, bool) { - query, err := url.ParseQuery(r.URL.RawQuery) - if err != nil { - return 0, false - } - +// messageCount returns the query's count, defaultCount if it has none, +// and false if it is anything but a whole number from 1 to maxCount. +func messageCount(query url.Values) (int, bool) { if !query.Has("count") { return defaultCount, true } diff --git a/internal/api/messages_test.go b/internal/api/messages_test.go index e5aede9..c56322b 100644 --- a/internal/api/messages_test.go +++ b/internal/api/messages_test.go @@ -123,7 +123,6 @@ func TestMessagesCount(t *testing.T) { "?count=2.5": 0, "?count=ten": 0, "?count=": 0, - "?count=1%": 0, } { t.Run(query, func(t *testing.T) { t.Parallel() @@ -151,6 +150,29 @@ func TestMessagesCount(t *testing.T) { } } +// TestMessagesUnreadableQuery: a query that cannot be decoded is refused +// with a sentence that says so, whether or not the bad part is count. +func TestMessagesUnreadableQuery(t *testing.T) { + t.Parallel() + + want := `{"error":"the query cannot be read"}` + "\n" + + for _, query := range []string{ + "?count=1%", "?count=5&x=%zz", "?x=%zz", "?count=5;x=1", + } { + client := &fakeClient{contacts: oneChat()} + rec := request(t, newAPI(credential, client), + http.MethodGet, messagesPath+query, bearer) + + if rec.Code != http.StatusBadRequest || rec.Body.String() != want || + client.count != 0 { + t.Errorf("%s: response = %d %q, asked for %d items; "+ + "want 400 %q and nothing asked", + query, rec.Code, rec.Body.String(), client.count, want) + } + } +} + // TestNoSuchChat: an id that GET /api/v1/chats does not list gets 404, // whether reading messages or sending one, and nothing is read or sent. // That includes 1 and 2, contact records the chat client keeps on a new