-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Refactor validatePictureUrl for better image URL validation #7287
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
base: master
Are you sure you want to change the base?
Conversation
Refactor validatePictureUrl function to improve URL validation and error handling for image URLs and Google Drive links. Signed-off-by: Safiya <[email protected]>
|
@kishore007k @leecalcote Please review the PR. |
|
|
||
| // Block base64 / data URIs | ||
| if (value.startsWith("data:")) { | ||
| return "Data URIs are not allowed. Please provide an image URL or Google Drive link."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not callout Google Drive, specifically.
"Data URI" is not user-friendly language either.
We simply need to state that a hyperlink to an image is what is required.
| "png", | ||
| "webp", | ||
| "svg", | ||
| "gif" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about avif?
leecalcote
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know that all of this validation is helpful, but is perhaps, more hurtful than anything else.
Do we need validation... at all? If so, the lighter, the better.
|
@safiya2610 why are you not reviewing the other open PR on this topic? |
Updated allowed image extensions to include 'avif'. Signed-off-by: Safiya <[email protected]>
|
Thanks for the feedback, @leecalcote |
surely I would take care of it. |
|
🚀 Preview for commit c69a476 at: https://694c21c40fcc296b5bee13b3--layer5.netlify.app |
Signed-off-by: Safiya <[email protected]>
|
@leecalcote I’ve updated the validation logic so that all valid URLs are accepted, including Drive and other hosted links. also keep it very simple and light. |
|
🚀 Preview for commit 985cae8 at: https://694c26f687b1ccb24b21826e--layer5.netlify.app |
Summary
Updated validatePictureUrl to support Google Drive links alongside standard image URLs.
Key Changes
Google Drive Integration: Added validation for /file/d/, open?id=, and uc?id= patterns.
Image Extensions: Maintained support for jpg, png, webp, svg, and gif.
Security: Blocked Base64/Data URIs to prevent large string uploads.
Reliability: Added try...catch with new URL() to handle malformed inputs gracefully.
Test Cases
Pass: Direct image URLs and Google Drive share links.
High Compatibility
Google Drive provides different types of links depending on how a user clicks "Share." By including all three, you ensure the user doesn't get a frustrating "Invalid URL" error just because they copied the link from a different menu:
/file/d/ID/view: The standard "Share" link most users copy.
open?id=ID: The older legacy sharing format.
uc?id=ID: The "Universal Content" link used for direct downloads and embedding.
Prevents "False Positives"
If you simply allowed any URL containing drive.google.com, a user could paste a link to a Google Drive Folder or the Drive Homepage. Your code ensures that only links pointing to individual files are accepted.
Fixes #6986