From 83f5bce12a8edcf702f914954c8b2f2bb4cea74e Mon Sep 17 00:00:00 2001 From: rostislav Date: Fri, 28 Feb 2025 14:05:47 +0200 Subject: [PATCH 1/5] #39353 Added validation for the presence of template variables in the user address --- .../Model/Address/Validator/General.php | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Customer/Model/Address/Validator/General.php b/app/code/Magento/Customer/Model/Address/Validator/General.php index 23c6d687328f3..3f13be77c1a8a 100644 --- a/app/code/Magento/Customer/Model/Address/Validator/General.php +++ b/app/code/Magento/Customer/Model/Address/Validator/General.php @@ -47,12 +47,36 @@ public function validate(AbstractAddress $address) { $errors = array_merge( $this->checkRequiredFields($address), - $this->checkOptionalFields($address) + $this->checkOptionalFields($address), + $this->checkTemplateVars($address) ); return $errors; } + /** + * Checking for the use of template variables + * + * @param AbstractAddress $address + * @return array + */ + private function checkTemplateVars(AbstractAddress $address) + { + $errors = []; + $pattern = '/\{\{\s*([^{}]+)\s*\}\}/'; + foreach ($address->getData() as $key => $value) { + preg_match_all($pattern, (string)$value, $matches); + if (!empty($matches[0])) { + $errors[] = __( + 'Invalid template variable detected in the "%fieldName" field. Utilization of {{ ... }} templates is prohibited', + ['fieldName' => $key] + ); + } + } + + return $errors; + } + /** * Check fields that are generally required. * From bd419faad3f5f4f8a80f3c3f818397a583ca08b4 Mon Sep 17 00:00:00 2001 From: rostislav Date: Fri, 28 Feb 2025 15:28:23 +0200 Subject: [PATCH 2/5] #39353 Copyright tweak --- app/code/Magento/Customer/Model/Address/Validator/General.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Customer/Model/Address/Validator/General.php b/app/code/Magento/Customer/Model/Address/Validator/General.php index 3f13be77c1a8a..8f2a1214f68ff 100644 --- a/app/code/Magento/Customer/Model/Address/Validator/General.php +++ b/app/code/Magento/Customer/Model/Address/Validator/General.php @@ -1,7 +1,7 @@ Date: Sun, 2 Mar 2025 16:04:17 +0200 Subject: [PATCH 3/5] #39353 Added sanitizer for template vars in address instead of validation --- .../Model/Address/AbstractAddress.php | 22 +++++- .../Model/Address/CompositeSanitizer.php | 41 ++++++++++ .../Model/Address/Sanitizer/TemplateVars.php | 78 +++++++++++++++++++ .../Model/Address/SanitizerInterface.php | 19 +++++ .../Model/Address/Validator/General.php | 30 +------ .../Model/ResourceModel/AddressRepository.php | 1 + app/code/Magento/Customer/etc/di.xml | 7 ++ 7 files changed, 169 insertions(+), 29 deletions(-) create mode 100644 app/code/Magento/Customer/Model/Address/CompositeSanitizer.php create mode 100644 app/code/Magento/Customer/Model/Address/Sanitizer/TemplateVars.php create mode 100644 app/code/Magento/Customer/Model/Address/SanitizerInterface.php diff --git a/app/code/Magento/Customer/Model/Address/AbstractAddress.php b/app/code/Magento/Customer/Model/Address/AbstractAddress.php index 51999970d2fbe..94b190ace6018 100644 --- a/app/code/Magento/Customer/Model/Address/AbstractAddress.php +++ b/app/code/Magento/Customer/Model/Address/AbstractAddress.php @@ -1,7 +1,7 @@ _directoryData = $directoryData; $data = $this->_implodeArrayField($data); @@ -206,6 +211,8 @@ public function __construct( ->get(CountryModelsCache::class); $this->regionModelsCache = $regionModelsCache ?: ObjectManager::getInstance() ->get(RegionModelsCache::class); + $this->compositeSanitizer = $compositeSanitizer ?: ObjectManager::getInstance() + ->get(CompositeSanitizer::class); parent::__construct( $context, $registry, @@ -680,6 +687,17 @@ public function validate() return $errors; } + /** + * Sanitize address attribute values. + * + * @return $this + */ + public function sanitize(): static + { + $this->compositeSanitizer->sanitize($this); + return $this; + } + /** * Create region instance * diff --git a/app/code/Magento/Customer/Model/Address/CompositeSanitizer.php b/app/code/Magento/Customer/Model/Address/CompositeSanitizer.php new file mode 100644 index 0000000000000..2753086671cb6 --- /dev/null +++ b/app/code/Magento/Customer/Model/Address/CompositeSanitizer.php @@ -0,0 +1,41 @@ +sanitizers = $sanitizers; + } + + /** + * @inheritdoc + */ + public function sanitize(AbstractAddress $address): AbstractAddress + { + foreach ($this->sanitizers as $sanitizer) { + $sanitizer->sanitize($address); + } + + return $address; + } +} diff --git a/app/code/Magento/Customer/Model/Address/Sanitizer/TemplateVars.php b/app/code/Magento/Customer/Model/Address/Sanitizer/TemplateVars.php new file mode 100644 index 0000000000000..1d7b480846f8a --- /dev/null +++ b/app/code/Magento/Customer/Model/Address/Sanitizer/TemplateVars.php @@ -0,0 +1,78 @@ +attributesToSanitize as $attributeCode) { + $attributeValue = $address->getData($attributeCode); + preg_match_all($this->templateVarsPattern, (string)$attributeValue, $matches); + if (!empty($matches[0])) { + $sanitizedAttributeValue = $this->sanitizeTemplateVars($attributeValue); + $address->setData($attributeCode, $sanitizedAttributeValue); + } + } + + return $address; + } + + /** + * Sanitize string for template vars in address attributes. + * + * @param string $value + * @return string + */ + private function sanitizeTemplateVars(string $value): string + { + if (!empty($value)) { + return str_replace(['{', '}'], ['{', '}'], $value); + } + return $value; + } + +} diff --git a/app/code/Magento/Customer/Model/Address/SanitizerInterface.php b/app/code/Magento/Customer/Model/Address/SanitizerInterface.php new file mode 100644 index 0000000000000..c9d0a202f3a78 --- /dev/null +++ b/app/code/Magento/Customer/Model/Address/SanitizerInterface.php @@ -0,0 +1,19 @@ +checkRequiredFields($address), - $this->checkOptionalFields($address), - $this->checkTemplateVars($address) + $this->checkOptionalFields($address) ); return $errors; } - /** - * Checking for the use of template variables - * - * @param AbstractAddress $address - * @return array - */ - private function checkTemplateVars(AbstractAddress $address) - { - $errors = []; - $pattern = '/\{\{\s*([^{}]+)\s*\}\}/'; - foreach ($address->getData() as $key => $value) { - preg_match_all($pattern, (string)$value, $matches); - if (!empty($matches[0])) { - $errors[] = __( - 'Invalid template variable detected in the "%fieldName" field. Utilization of {{ ... }} templates is prohibited', - ['fieldName' => $key] - ); - } - } - - return $errors; - } - /** * Check fields that are generally required. * diff --git a/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php b/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php index fed467a6c2a4b..089c3d9647825 100644 --- a/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php +++ b/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php @@ -126,6 +126,7 @@ public function save(\Magento\Customer\Api\Data\AddressInterface $address) $addressModel->setStoreId($customerModel->getStoreId()); } + $addressModel = $addressModel->sanitize(); $errors = $addressModel->validate(); if ($errors !== true) { $inputException = new InputException(); diff --git a/app/code/Magento/Customer/etc/di.xml b/app/code/Magento/Customer/etc/di.xml index 04dee8d9f6b63..1265d0b79dbdb 100644 --- a/app/code/Magento/Customer/etc/di.xml +++ b/app/code/Magento/Customer/etc/di.xml @@ -472,6 +472,13 @@ + + + + Magento\Customer\Model\Address\Sanitizer\TemplateVars + + + customer_group From f33deefcf0639a2a4e3cac296f011360777ff1d3 Mon Sep 17 00:00:00 2001 From: rostislav Date: Sun, 2 Mar 2025 23:51:40 +0200 Subject: [PATCH 4/5] #39353 Static checks bug fixes, unit tests added --- .../Model/Address/AbstractAddress.php | 2 +- .../Model/Address/CompositeSanitizer.php | 3 +-- .../Model/Address/Sanitizer/TemplateVars.php | 2 -- .../Model/Address/SanitizerInterface.php | 3 ++- .../Model/ResourceModel/AddressRepository.php | 4 ++-- .../ResourceModel/AddressRepositoryTest.php | 23 +++++++++++++++++-- app/code/Magento/Customer/etc/di.xml | 4 ++-- 7 files changed, 29 insertions(+), 12 deletions(-) diff --git a/app/code/Magento/Customer/Model/Address/AbstractAddress.php b/app/code/Magento/Customer/Model/Address/AbstractAddress.php index 94b190ace6018..1275e73cc20b0 100644 --- a/app/code/Magento/Customer/Model/Address/AbstractAddress.php +++ b/app/code/Magento/Customer/Model/Address/AbstractAddress.php @@ -692,7 +692,7 @@ public function validate() * * @return $this */ - public function sanitize(): static + public function sanitize(): self { $this->compositeSanitizer->sanitize($this); return $this; diff --git a/app/code/Magento/Customer/Model/Address/CompositeSanitizer.php b/app/code/Magento/Customer/Model/Address/CompositeSanitizer.php index 2753086671cb6..8e7e8bdb944b9 100644 --- a/app/code/Magento/Customer/Model/Address/CompositeSanitizer.php +++ b/app/code/Magento/Customer/Model/Address/CompositeSanitizer.php @@ -22,8 +22,7 @@ class CompositeSanitizer implements SanitizerInterface */ public function __construct( array $sanitizers = [] - ) - { + ) { $this->sanitizers = $sanitizers; } diff --git a/app/code/Magento/Customer/Model/Address/Sanitizer/TemplateVars.php b/app/code/Magento/Customer/Model/Address/Sanitizer/TemplateVars.php index 1d7b480846f8a..026168cdfdde4 100644 --- a/app/code/Magento/Customer/Model/Address/Sanitizer/TemplateVars.php +++ b/app/code/Magento/Customer/Model/Address/Sanitizer/TemplateVars.php @@ -40,7 +40,6 @@ class TemplateVars implements SanitizerInterface 'vat_id' ]; - /** * Sanitize string for template vars in address attributes. * @@ -74,5 +73,4 @@ private function sanitizeTemplateVars(string $value): string } return $value; } - } diff --git a/app/code/Magento/Customer/Model/Address/SanitizerInterface.php b/app/code/Magento/Customer/Model/Address/SanitizerInterface.php index c9d0a202f3a78..6bd409fac3768 100644 --- a/app/code/Magento/Customer/Model/Address/SanitizerInterface.php +++ b/app/code/Magento/Customer/Model/Address/SanitizerInterface.php @@ -3,6 +3,7 @@ * Copyright 2025 Adobe * All Rights Reserved. */ +declare(strict_types=1); namespace Magento\Customer\Model\Address; @@ -12,7 +13,7 @@ interface SanitizerInterface * Sanitize address instance. * * @param AbstractAddress $address - * @return AbstractAddress $address + * @return AbstractAddress * @since 102.0.0 */ public function sanitize(AbstractAddress $address): AbstractAddress; diff --git a/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php b/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php index 089c3d9647825..027a4824a3ccb 100644 --- a/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php +++ b/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php @@ -1,7 +1,7 @@ address->expects($this->once()) ->method('setCustomer') ->with($this->customer); + $this->address->expects($this->once()) + ->method('sanitize') + ->willReturn($this->address); $this->address->expects($this->once()) ->method('validate') ->willReturn(true); @@ -280,6 +284,9 @@ public function testSaveWithConfigCustomerAccountShareScopeWebsite() ->willReturn(true); $this->address->expects($this->once()) ->method('setStoreId'); + $this->address->expects($this->once()) + ->method('sanitize') + ->willReturn($this->address); $this->address->expects($this->once()) ->method('validate') ->willReturn(true); @@ -350,6 +357,9 @@ public function testSaveWithConfigCustomerAccountShareScopeGlobal() ->willReturn(false); $this->address->expects($this->never()) ->method('setStoreId'); + $this->address->expects($this->once()) + ->method('sanitize') + ->willReturn($this->address); $this->address->expects($this->once()) ->method('validate') ->willReturn(true); @@ -404,6 +414,9 @@ public function testSaveWithException() $this->address->expects($this->once()) ->method('updateData') ->with($customerAddress); + $this->address->expects($this->once()) + ->method('sanitize') + ->willReturn($this->address); $this->address->expects($this->once()) ->method('validate') ->willReturn($errors); @@ -446,6 +459,9 @@ public function testSaveWithInvalidRegion() $this->address->expects($this->never()) ->method('getRegionId') ->willReturn(null); + $this->address->expects($this->once()) + ->method('sanitize') + ->willReturn($this->address); $this->address->expects($this->once()) ->method('validate') ->willReturn($errors); @@ -487,6 +503,9 @@ public function testSaveWithInvalidRegionId() $this->address->expects($this->never()) ->method('getRegion') ->willReturn(''); + $this->address->expects($this->once()) + ->method('sanitize') + ->willReturn($this->address); $this->address->expects($this->once()) ->method('validate') ->willReturn($errors); diff --git a/app/code/Magento/Customer/etc/di.xml b/app/code/Magento/Customer/etc/di.xml index 1265d0b79dbdb..31065ce700c4c 100644 --- a/app/code/Magento/Customer/etc/di.xml +++ b/app/code/Magento/Customer/etc/di.xml @@ -1,8 +1,8 @@ From ca28932e2b6cbea8db416f763ac7f66a0209123f Mon Sep 17 00:00:00 2001 From: Rostislav Sulejmanov <85498741+rostilos@users.noreply.github.com> Date: Thu, 3 Apr 2025 13:30:22 +0300 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Abhinav Pathak <51681618+engcom-Hotel@users.noreply.github.com> --- app/code/Magento/Customer/Model/Address/AbstractAddress.php | 2 +- app/code/Magento/Customer/Model/Address/Validator/General.php | 2 +- .../Magento/Customer/Model/ResourceModel/AddressRepository.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Customer/Model/Address/AbstractAddress.php b/app/code/Magento/Customer/Model/Address/AbstractAddress.php index 1275e73cc20b0..47aaabbf4aa31 100644 --- a/app/code/Magento/Customer/Model/Address/AbstractAddress.php +++ b/app/code/Magento/Customer/Model/Address/AbstractAddress.php @@ -1,6 +1,6 @@