fix: oversized message cursors (#15400)
Message pagination now constrains client-provided cursors to the PostgreSQL integer range used by `messages.id`, preventing oversized values from raising database errors while preserving before/after pagination semantics. ## Closes - [CW-7922](https://linear.app/chatwoot/issue/CW-7922/harden-backend-paths-causing-production-sentry-errors) - [Sentry 7663397140](https://chatwoot-p3.sentry.io/issues/7663397140/) - [Sentry 7663397546](https://chatwoot-p3.sentry.io/issues/7663397546/) - [Sentry 7663464772](https://chatwoot-p3.sentry.io/issues/7663464772/) ## How to reproduce Request conversation messages with an extremely large `before` or `after` cursor, such as `4611686018427387903`. PostgreSQL previously rejected the value as outside the range for the integer `messages.id` column. ## What changed - Normalize message cursors before they reach the query. - Clamp them to the valid signed 32-bit integer ID range. - Cover oversized `before` and `after` cursors with finder specs.
This commit is contained in:
@@ -1,4 +1,6 @@
|
||||
class MessageFinder
|
||||
MESSAGE_ID_MAX = 2_147_483_647
|
||||
|
||||
def initialize(conversation, params)
|
||||
@conversation = conversation
|
||||
@params = params
|
||||
@@ -21,12 +23,14 @@ class MessageFinder
|
||||
end
|
||||
|
||||
def current_messages
|
||||
return messages.none if oversized_message_id?(@params[:after])
|
||||
|
||||
if @params[:after].present? && @params[:before].present?
|
||||
messages_between(@params[:after].to_i, @params[:before].to_i)
|
||||
messages_between(normalized_message_id(@params[:after]), @params[:before].to_i)
|
||||
elsif @params[:before].present?
|
||||
messages_before(@params[:before].to_i)
|
||||
elsif @params[:after].present?
|
||||
messages_after(@params[:after].to_i)
|
||||
messages_after(normalized_message_id(@params[:after]))
|
||||
else
|
||||
messages_latest
|
||||
end
|
||||
@@ -37,16 +41,29 @@ class MessageFinder
|
||||
end
|
||||
|
||||
def messages_before(before_id)
|
||||
return messages_latest if oversized_message_id?(before_id)
|
||||
|
||||
before_id = normalized_message_id(before_id)
|
||||
messages.reorder('created_at desc').where('id < ?', before_id).limit(20).reverse
|
||||
end
|
||||
|
||||
def messages_between(after_id, before_id)
|
||||
messages.reorder('created_at asc').where('id >= ? AND id < ?', after_id, before_id).limit(1000)
|
||||
message_scope = messages.reorder('created_at asc').where('id >= ?', after_id)
|
||||
message_scope = message_scope.where('id < ?', normalized_message_id(before_id)) unless oversized_message_id?(before_id)
|
||||
message_scope.limit(1000)
|
||||
end
|
||||
|
||||
def messages_latest
|
||||
messages.reorder('created_at desc').limit(20).reverse
|
||||
end
|
||||
|
||||
def normalized_message_id(value)
|
||||
value.to_i.clamp(0, MESSAGE_ID_MAX)
|
||||
end
|
||||
|
||||
def oversized_message_id?(value)
|
||||
value.to_i > MESSAGE_ID_MAX
|
||||
end
|
||||
end
|
||||
|
||||
MessageFinder.prepend_mod_with('MessageFinder')
|
||||
|
||||
@@ -48,6 +48,17 @@ describe MessageFinder do
|
||||
end
|
||||
end
|
||||
|
||||
context 'with a before attribute above the message id range' do
|
||||
let!(:max_id_message) do
|
||||
create(:message, id: described_class::MESSAGE_ID_MAX, account: account, inbox: inbox, conversation: conversation)
|
||||
end
|
||||
let(:params) { { before: 4_611_686_018_427_387_903 } }
|
||||
|
||||
it 'includes the maximum valid message id without overflowing the database column' do
|
||||
expect(message_finder.perform).to include(max_id_message)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with after attribute' do
|
||||
let(:params) { { after: conversation.messages.first.id } }
|
||||
|
||||
@@ -59,6 +70,14 @@ describe MessageFinder do
|
||||
end
|
||||
end
|
||||
|
||||
context 'with an after attribute above the message id range' do
|
||||
let(:params) { { after: 881_965_304_328 } }
|
||||
|
||||
it 'returns no messages without overflowing the database column' do
|
||||
expect(message_finder.perform).to be_empty
|
||||
end
|
||||
end
|
||||
|
||||
context 'with after and before attribute' do
|
||||
let(:params) do
|
||||
{
|
||||
@@ -73,5 +92,24 @@ describe MessageFinder do
|
||||
expect(result.last.id).to be conversation.messages[-2].id
|
||||
end
|
||||
end
|
||||
|
||||
context 'with after and before attributes above the message id range' do
|
||||
let!(:max_id_message) do
|
||||
create(:message, id: described_class::MESSAGE_ID_MAX, account: account, inbox: inbox, conversation: conversation)
|
||||
end
|
||||
let(:params) do
|
||||
{
|
||||
after: 881_965_304_328,
|
||||
before: 4_611_686_018_427_387_903
|
||||
}
|
||||
end
|
||||
|
||||
it 'returns no messages' do
|
||||
result = message_finder.perform
|
||||
|
||||
expect(result).to be_empty
|
||||
expect(result).not_to include(max_id_message)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user