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

[5.1] Form: Add regex validation rule #42657

Merged
merged 4 commits into from
Feb 28, 2024

Conversation

Hackwar
Copy link
Member

@Hackwar Hackwar commented Jan 14, 2024

Summary of Changes

This PR adds a regex validation rule for the Form library. The base class for Form validation rules has validation with regular expression implemented, but no way to hand in a regular expression via the form definition. You have to build your own validation rule class, register it in Joomla and can then use it. That seems overly complex for this, so this PR adds a rule to add regular expression validation.

For your field, you add validate="regex" to your form definition and the regular expression via validate_regex="". The value of the field is then validated server side against the regex given. You have to pay attention that you might have to escape the regex properly when adding it to the validate_regex attribute. You can also add a modifier at the end.

Testing Instructions

  1. Create a new instance of a core module, for example mod_custom in the backend and go to the "Advanced" tab. Modify the value of one of the entries in the "Layout" select to be a value which does not contain A-Z, a number, - or _. Invalid would for example be üüü.
  2. When saving, you should get an error, that this is invalid.
  3. Edit /modules/mod_custom/mod_custom.xml and replace validate="moduleLayout" with
    				validate="regex"
    				validate_regex="^([A-Za-z0-9_-]+:)?[A-Za-z0-9-][A-Za-z0-9\.-]*$"
    
  4. Try the same as in 1. and see that it still fails, but that the other values still properly pass.

Or it could be approved by maintainers by codereview.

Thanks

A big thanks goes to @coolcat-creations (Website) who sponsored this PR.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org: Form: Serverside validation rules Manual#228

  • No documentation changes for manual.joomla.org needed

@webnet-assmann
Copy link

Hello Hannes,
I've tested it, but I don't know if I understood it correctly:
You have to select something in the Advanced tab under Layout and enter a value with mutated vowel. I can't change the values there, so I created another file for Layout with üüü and then selected that. Then I got the error message. Then I changed the corresponding code in the mod_custom.xml file and selected üüü again and the error message appeared again, but the other values could be saved.
If I have understood correctly, then the test was successful.

@Hackwar
Copy link
Member Author

Hackwar commented Jan 15, 2024

Thanks @webnet-assmann. Yes, that would be a successfull test. The goal was to show that the regex validation rule properly functions, regardless if we use the specific rule for moduleLayout or if we copy the regex from there into the XML.

@webnet-assmann
Copy link

Tested successfully


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42657.

@richard67
Copy link
Member

Tested successfully

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42657.

@webnet-assmann Could you mark your test result in the issue tracker so it’s properly counted? Just go to https://issues.joomla.org/tracker/joomla-cms/42657 , use the blue „Test this“ button at the top left corner, then select your t st result and submit. Thanks in advance.

@brianteeman
Copy link
Contributor

This really needs sopme documentation please

@webnet-assmann
Copy link

I have tested this item ✅ successfully on 70a3a53

Tested successfully


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42657.

@coolcat-creations
Copy link
Contributor

What would be the right place to add the documentation before the PR is merged?

@brianteeman
Copy link
Contributor

but no way to hand in a regular expression via the form definition.

Is that really correct? You can use html5 pattern already and from what I can see it does the same thing and is something we already use. What does this offer that is different?

@coolcat-creations
Copy link
Contributor

For example:

  • Email Filtering: Exclude specific domains (e.g., block freemail accounts). For example exclude gmail and web.de
  • ZIP Code Validation: Ensure format consistency for different countries. For example 5 Numbers in Germany
  • IBAN Validation: Validate international bank account numbers.
  • Telephone Numbers: Accommodate country-specific formats and codes. For example +49 170 1234456 against (0170) 1234456
  • Social Security Numbers: Validate format based on country-specific standards.
  • Vehicle License Plates: Match specific national or regional patterns.
  • Currency Formatting: Ensure correct currency symbol and format.
  • Time Formats: Validate time in various international formats in a text field For example: 10.00–18.00 Uhr
  • Postal Address Formatting: Check for country-specific postal address formats.

@brianteeman
Copy link
Contributor

@coolcat-creations All of that is already possible using pattern (https://www.html5pattern.com/) or am I missing some big difference that requires this extra code

@Fedik
Copy link
Member

Fedik commented Jan 15, 2024

@brianteeman html5 attribute "pattern" is client side, what is in PR is server side.
It is probably possible to use the same pattern for both client and server (and so simplify the field set up), but I did not tested such thing, cannot say for sure.

@brianteeman
Copy link
Contributor

@Fedik is that enough to justify this. Seems overkill to me to add this as its almost duplicate functionality

@Fedik
Copy link
Member

Fedik commented Jan 15, 2024

Seems overkill to me to add this as its almost duplicate functionality

Well, yes and no.
You know, you cannot rely on client side validation, never.

@brianteeman
Copy link
Contributor

agree when it is security related such as with a password but in these examples client side is much faster and provides a better user experience

@Hackwar
Copy link
Member Author

Hackwar commented Jan 15, 2024

@brianteeman I'm happy to discuss improvements on the client side in the future, but we need server side validation and this is such a server side validation. If you want to get rid of duplicate functionality, we could rather talk about removing the validation rules we have right now which only contain such a regex. However, there are good reasons for providing even such basic rules like Boleean or ModuleLayout.

@wilsonge
Copy link
Contributor

agree when it is security related such as with a password but in these examples client side is much faster and provides a better user experience

We basically need both. Server side is always going to be required regardless (if nothing else we have an API still to maintain which can't do HTML5 validation). Many of the security issues we had in 3.9/3.10 were people intercepting requests from forms to the server and then making further modifications because we weren't doing proper server side validation (only client side). For regex things it's hard to say what the business implications might be - but I'm sure there's going to be some business cases things like phone numbers might be required.

TLDR: I think this is fine - combining the server and client side regex together into a single field if provided definitely a nice to have though.

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 70a3a53


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42657.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42657.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 24, 2024
@LadySolveig LadySolveig added this to the Joomla! 5.1.0 milestone Feb 28, 2024
@Hackwar
Copy link
Member Author

Hackwar commented Feb 28, 2024

I added documentation for all validation rules to the manual. Hope that is enough like this.

@bembelimen bembelimen merged commit 1954f0a into joomla:5.1-dev Feb 28, 2024
0 of 2 checks passed
@bembelimen
Copy link
Contributor

Thx

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 28, 2024
@Hackwar Hackwar deleted the 5.1-customfields1 branch March 4, 2024 17:00
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.