Skip to content

IBX-9217: Added URL validation #1418

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

Merged
merged 2 commits into from
Mar 11, 2025
Merged

IBX-9217: Added URL validation #1418

merged 2 commits into from
Mar 11, 2025

Conversation

OstafinL
Copy link
Contributor

@OstafinL OstafinL commented Jan 14, 2025

🎫 Issue IBX-9217

Description:

It is base validation without escaping because IBX-8962 ibexa/fieldtype-richtext#202

For QA:

Ad. 9e00eb2 / 5ad899b

In the end modified scenarios for variants in https://github.com/ibexa/product-catalog/pull/1251 Added retrying (and mouse over) for switching field groups due to random error encountered on Chrome v124 on Headless:

https://github.com/ibexa/headless/actions/runs/13391931955/job/37401422810#step:18:215
https://res.cloudinary.com/ezplatformtravis/image/upload/v1739887300/screenshots/67b492c43b022776389969-vendor_ibexa_product-catalog_features_browser_productvariants_feature_165_whc2qm.png

In this PR tests fail because Chrome 110 is used (without support for canParse).
Tests with Chrome 124 are run ibexa/commerce#1186, ibexa/experience#520, ibexa/headless#190, ibexa/oss#201.

Documentation:

@OstafinL OstafinL added Bug Something isn't working Ready for review labels Jan 14, 2025
@OstafinL OstafinL changed the base branch from main to 4.6 January 14, 2025 09:26
@glye glye self-requested a review January 14, 2025 09:53
Copy link
Contributor

@glye glye left a comment

Choose a reason for hiding this comment

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

As I understand it, this will approve all valid URLs no matter the protocol. If we later want to introduce an FT setting to only allow certain protocols, we can support that here by checking the url protocol property.

@OstafinL
Copy link
Contributor Author

As I understand it, this will approve all valid URLs no matter the protocol. If we later want to introduce an FT setting to only allow certain protocols, we can support that here by checking the url protocol property.

In this implementation, I’m checking if string is a valid url, no matter what the protocol is, as long as it is actually a protocol :) but we can later add additional protocol validation via property protocol of URL

const parsedUrl = new URL(urlValue); parsedUrl.protocol

result.errorMessage = ibexa.errors.emptyField.replace('{fieldName}', label);
}

if (!isEmpty) {
try {
new URL(urlValue);
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of doing this whole try catch you can do normal if using static method canParse https://developer.mozilla.org/en-US/docs/Web/API/URL/canParse_static

@OstafinL OstafinL force-pushed the IBX-9217-URL-validation branch 2 times, most recently from 7142014 to c768afb Compare February 11, 2025 09:39
@micszo micszo force-pushed the IBX-9217-URL-validation branch from 5ad899b to c768afb Compare February 24, 2025 08:12
@micszo micszo force-pushed the IBX-9217-URL-validation branch from c768afb to 2153682 Compare February 27, 2025 11:52
@juskora juskora force-pushed the IBX-9217-URL-validation branch from 2153682 to 9ff1f74 Compare March 5, 2025 07:21
@juskora
Copy link
Contributor

juskora commented Mar 5, 2025

@OstafinL
When typing invalid URL, the validation works, however, when invalid URLs are copied https://example.com/path/page[1, https://http://example.com,

2025-03-05_10-34-36.mp4

nothing happens, see recording.

@juskora
Copy link
Contributor

juskora commented Mar 6, 2025

QA Approved on Ibexa DXP Commerce 4.6.x-dev and 4.6.17.
Additional comments:
It fixed validation partially, the rest is dependent on https://issues.ibexa.co/browse/IBX-8962 and waits for the fix.

@OstafinL OstafinL force-pushed the IBX-9217-URL-validation branch from 82a9d92 to 0cbce5a Compare March 7, 2025 07:46
@OstafinL OstafinL changed the base branch from 4.6 to main March 7, 2025 07:48
@OstafinL OstafinL force-pushed the IBX-9217-URL-validation branch from 0cbce5a to 02e3ec3 Compare March 7, 2025 07:54
@OstafinL OstafinL changed the base branch from main to 4.6 March 7, 2025 07:55
Copy link

sonarqubecloud bot commented Mar 7, 2025

@dew326 dew326 merged commit eb1065a into 4.6 Mar 11, 2025
24 of 25 checks passed
@dew326 dew326 deleted the IBX-9217-URL-validation branch March 11, 2025 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.