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: 500/ dont expose invalid base64 decode error for page token #1314

Merged
merged 2 commits into from
Feb 3, 2023

Conversation

markphelps
Copy link
Collaborator

@markphelps markphelps commented Feb 2, 2023

Fixes: FLI-189

  • Prevent 500
  • Don't expose invalid base64 decode error to user if bad pageToken

Before

curl -v "http://localhost:8080/api/v1/flags/flagKey/rules?limit=10&offset=10&pageToken=pageToken"

< HTTP/1.1 500 Internal Server Error
< Content-Type: application/json
< Vary: Origin
< X-Content-Type-Options: nosniff
< Date: Fri, 27 Jan 2023 20:26:02 GMT
< Content-Length: 72

{"code":13,"message":"illegal base64 data at input byte 8","details":[]}%     

After

curl -v "http://localhost:8080/api/v1/flags/flagKey/rules?limit=10&offset=10&pageToken=pageToken"

< HTTP/1.1 400 Bad Request
< Content-Type: application/json
< Vary: Origin
< Date: Thu, 02 Feb 2023 21:16:11 GMT
< Content-Length: 70

{"code":3,"message":"pageToken is not valid: \"pageToken\"","details":[]}%     

@markphelps markphelps requested a review from a team as a code owner February 2, 2023 21:12
@markphelps markphelps changed the title fix: dont expose invalid base64 decode error for page token fix: 500/ dont expose invalid base64 decode error for page token Feb 2, 2023
Copy link
Contributor

@GeorgeMac GeorgeMac left a comment

Choose a reason for hiding this comment

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

Lovely.

@markphelps markphelps merged commit e495c91 into main Feb 3, 2023
@markphelps markphelps deleted the handle-bad-page-token branch February 3, 2023 13:08
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