39854: Fix city name validation to allow digits, &, ., () characters

[bhushan-cs]

Description (*)

Fixed city name validation to allow commonly used characters that were being incorrectly rejected. The current validation was too restrictive and prevented valid city names containing numbers, periods, ampersands, and parentheses.

Fixed Issues (if relevant)

1. Fixes [2.4.8] Cannot place orders where City has digits 0-9, Ampersand, full stop or parenthesis in the City Name #39854

Manual testing scenarios (*)

1. Try to place an order with city names like:
   • "St. Petersburg"
   • "Trinidad & Tobago"
   • "Paris (75001)"
2. Verify checkout completes successfully

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @bhushan-cs. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

[gwharton]

You might want to update the error message as well so users are informed of the correct characters that can be used.

[bhushan-cs]

@gwharton thanks for the feedback! That's a great point. I think the following will be good. What do you think?
Invalid City. Please use letters, numbers, spaces, and the following characters: - ' . , & ( )

[gwharton]

Looks good to me.

I may expect some push back from Magento team on this, as they explicitly used to allow commas, then they removed commas as allowed in 2.4.8-beta1, so I'm figuring there must have been a reason why they tightened this down. They'll have to advise.

I would be willing to concede that comma does not belong in the city name, as comma is commonly used as a separator for address fields, but I can't see a reason why the others shouldn't be allowed, especially ampersand and full stop which should 100% be allowed IMHO.

[hostep]

@gwharton, it wasn't Magento core devs that added this check, it was done by the community in #38345, more specifically these comments seems relevant: #38345 (comment) & #38345 (comment)

So maybe @dekiakbar & @Franciscof-Serfe can take a look here and leave some feedback?

[gwharton]

Yep, looks like a discussion around security is needed again. There must be a better way of detecting someone entering code snippets into the address fields without banning full stop, comma, ampersand, parenthesis and digits.

None of my customers who live in "Brighton & Hove" city can place orders with me without misspelling their city name (using and instead of &). Same applies for my customers in "St. Davids" and "Ventnor, Isle of Wight"

[bhushan-cs]

Thanks for digging into the history, @hostep and @gwharton
I agree that a solution that allows common characters like '&' and '.' while preventing malicious input would be ideal.
I'm interested to hear @dekiakbar & @Franciscof-Serfe's thoughts on this.
In the meantime, I'll keep the updated error message in place.

[Franciscof-Serfe]

Hi @ALL ,
The expression /^[\p{L}\p{M}\d\s-'.,&()]{1,100}$/u proposed by @bhushan-cs is robust and covers a wide range of valid characters for city names. That said, I would suggest adding support for the underscore (_) and typographical apostrophe (’) for more flexibility, especially considering real-world edge cases like "O'Higgins".

This validation pattern was originally introduced by @dekiakbar during a period of increased fraudulent activity. Attackers were submitting real credit card data in an attempt to detect vulnerabilities. These were sanitized inputs, but the attempts were annoying. At the time, I reviewed and validated the approach, suggesting applying the same pattern to other input fields for consistency.

I agree that we don't need to be overly strict to filter out malicious input while still accepting legitimate data. This small adjustment keeps us on that path.

[gwharton]

Hi @bhushan-cs Could you update according with the additional suggestions above, and also add the appropriate test cases to Magento/Customer/Test/Unit/Model/Validator/CityTest.php to verify each of the new additions.

I do have a slight worry that the original case of preventing the injection of code snippets into address fields is now negated by this change. However, provided the address fields are escaped internally by Magento, which i would hope they are, is there any risk of arbitrary code execution, even if a code snippet was injected.

[dekiakbar]

Hi All,

@hostep Yes, its from that change. I agree, my changes was too strict and we should not to make it strict like this

@bhushan-cs Your changes have definitely improved the validation, and the updated error message is much clearer and easier to understand. I fully agree with your approach — well done

@gwharton You're absolutely right, and I’m really sorry for the inconvenience this has caused. I completely agree — blocking common characters like &, ., () and digits in address fields isn’t practical, especially when they’re part of legitimate place names like "Brighton & Hove" or "St. Davids."

[gwharton]

For those stores that have individual and local issues, I guess there's nothing stopping them overriding the existing city validator in the XML config and having a stricter version local to their store.

[dekiakbar]

That's a great point, allowing stores to override the city validator locally makes a lot of sense. It gives flexibility for specific needs without affecting the global behavior

[bhushan-cs]

Hi @Franciscof-Serfe, @gwharton and @dekiakbar,

Thanks for the further feedback and suggestions!

@Franciscof-Serfe, I agree that adding support for underscore (_) and the typographical apostrophe (’) would definitely improve flexibility and handle more real-world city names accurately. I've updated the regular expression to include these.

@gwharton, I've also added the appropriate test cases to ensure these new additions are covered.