Skip to content

Validate jsonp callbacks, add unit tests#4030

Merged
ozh merged 4 commits intomasterfrom
sanitize_jsonp
Dec 22, 2025
Merged

Validate jsonp callbacks, add unit tests#4030
ozh merged 4 commits intomasterfrom
sanitize_jsonp

Conversation

@ozh
Copy link
Copy Markdown
Member

@ozh ozh commented Dec 13, 2025

No description provided.

@ozh ozh requested a review from a team December 13, 2025 14:57
@dgw dgw mentioned this pull request Dec 17, 2025
4 tasks
@dgw dgw self-assigned this Dec 17, 2025
Copy link
Copy Markdown
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

I have a small nitpick, but also a fundamental question: Why not simply reject invalid JSONP callbacks with an error?

As I understand JSONP, whether YOURLS explicitly returns an error response or silently modifies the callback name, the calling script won't do what's expected. Might as well make the reason obvious, IMO.

@ozh
Copy link
Copy Markdown
Member Author

ozh commented Dec 17, 2025

the calling script won't do what's expected. Might as well make the reason obvious, IMO

Absolutely. Will change this 👍

ozh and others added 2 commits December 20, 2025 16:52
Remove formatting caused by cat stepping on keyboard

Co-authored-by: dgw <dgw@technobabbl.es>
Return explicit error message and HTTP status code if invalid
Copy link
Copy Markdown
Member Author

@ozh ozh left a comment

Choose a reason for hiding this comment

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

Good to go for me

Copy link
Copy Markdown
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

🚀

@dgw dgw changed the title Sanitize jsonp callbacks, add unit tests Validate jsonp callbacks, add unit tests Dec 22, 2025
@ozh ozh merged commit b1c6100 into master Dec 22, 2025
10 checks passed
@ozh ozh deleted the sanitize_jsonp branch December 22, 2025 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants