search

waku-org / nwaku

Waku node and protocol.
Other
183 stars 46 forks source link

bug: store should validate that the cursor used in the request is valid #2653

Open richard-ramos opened 2 weeks ago

richard-ramos commented 2 weeks ago

Problem

If a store request is created with an invalid cursor (that's is correctly formated), like this one: 0x0000000000000000000000000000000000000000000000000000000000000000:

{
  "request_id": "ec560c592484291ca6dfa1491507a45dba55b238c2b271cf0cb81ce1e0590fce",
  "include_data": true,
  "pubsub_topic": "/waku/2/default-waku/proto",  // TODO: replace this 
  "content_topics": ["some_content_topic"],      // TODO: replace this 
  "time_start": 1714155079182460704,             // TODO: replace this 
  "time_end": 1714155079285053447,               // TODO: replace this 
  "pagination_cursor": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=",
  "pagination_forward": true,
  "pagination_limit": 20
}

Store request is not validating that the cursor that was sent does not exist. The correct behavior should be to return either no results, or an invalid_cursor error. I think we should do the later.

This can be solved by doing a SELECT EXISTS(SELECT 1 FROM messages WHERE message_hash=?) to determine if the cursor exists. Since the message_hash is the primary key of the table, I imagine this should execute fast with no major impact on the total store query response time

This affects both storeV2 and V3.

Ivansete-status commented 2 weeks ago

@richard-ramos - I set it as ToDo because I consider that any cursor used by any client might be previously given by the store node. With that, I assume that this won't have very big impact, unless I'm missing sth :)

richard-ramos commented 1 week ago

I think it might become an issue if you're retrieving pages, and suddenly the retention policy is applied and the cursor the user has is no longer valid. The user would proceed to use this cursor and end up receiving the list of messages again starting from record 0.