Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: URL encoding for pubsubTopic and contentTopics parameters #3200

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vishwamartur
Copy link

Related to #3128

Update the API to enforce mandatory URL encoding for pubsubTopic and content_topic.

  • Update docs/api/rest-api.md to include examples of URL-encoded pubsubTopic and contentTopics parameters.
  • Modify waku/waku_api/rest/store/handlers.nim to validate and enforce URL encoding for pubsubTopic and contentTopics parameters.
  • Add error handling for invalid or non-encoded pubsubTopic and contentTopics parameters in waku/waku_api/rest/store/handlers.nim.
  • Update decodeRequestBody function in waku/waku_api/rest/rest_serdes.nim to validate and enforce URL encoding for pubsubTopic and contentTopics parameters.
  • Add error handling for invalid or non-encoded pubsubTopic and contentTopics parameters in waku/waku_api/rest/rest_serdes.nim.

Related to waku-org#3128

Update the API to enforce mandatory URL encoding for `pubsubTopic` and `content_topic`.

* Update `docs/api/rest-api.md` to include examples of URL-encoded `pubsubTopic` and `contentTopics` parameters.
* Modify `waku/waku_api/rest/store/handlers.nim` to validate and enforce URL encoding for `pubsubTopic` and `contentTopics` parameters.
* Add error handling for invalid or non-encoded `pubsubTopic` and `contentTopics` parameters in `waku/waku_api/rest/store/handlers.nim`.
* Update `decodeRequestBody` function in `waku/waku_api/rest/rest_serdes.nim` to validate and enforce URL encoding for `pubsubTopic` and `contentTopics` parameters.
* Add error handling for invalid or non-encoded `pubsubTopic` and `contentTopics` parameters in `waku/waku_api/rest/rest_serdes.nim`.
@Ivansete-status Ivansete-status changed the title Fix URL encoding for pubsubTopic and contentTopics parameters fix: URL encoding for pubsubTopic and contentTopics parameters Dec 9, 2024
Copy link
Collaborator

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I just added a few comments/question

if T.hasKey("pubsubTopic"):
let pubsubTopic = T["pubsubTopic"]
if pubsubTopic != encodeUrl(pubsubTopic):
return err(RestApiResponse.badRequest("Invalid or non-encoded pubsubTopic parameter"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return err(RestApiResponse.badRequest("Invalid or non-encoded pubsubTopic parameter"))
return err(RestApiResponse.badRequest("Invalid or non-URL-encoded pubsubTopic parameter"))

let contentTopics = T["contentTopics"]
for topic in contentTopics:
if topic != encodeUrl(topic):
return err(RestApiResponse.badRequest("Invalid or non-encoded content_topic parameter"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return err(RestApiResponse.badRequest("Invalid or non-encoded content_topic parameter"))
return err(RestApiResponse.badRequest("Invalid or non-URL-encoded content_topic parameter"))

#### [`get_waku_v2_store_v3_messages`](https://rfc.vac.dev/spec/16/#get_waku_v2_store_v3_messages)

```bash
curl -v -X GET "http://127.0.0.1:49153/store/v3/messages?includeData=true&pubsubTopic=/waku/2/rs/3/0&pageSize=20&ascending=true"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that it still works with that format :) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants