From 0d349ea67073c535e1aa7f19f3cf842a54458bfe Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 8 Jun 2017 21:51:46 +0200 Subject: Validate language & preview picture fields Instead of saving the value of each field right into the content without any validation, it seems better to validate them. This might sounds obvious now we say that. --- src/Wallabag/CoreBundle/Helper/ContentProxy.php | 78 +++++++++++++++++++--- .../CoreBundle/Resources/config/services.yml | 1 + 2 files changed, 68 insertions(+), 11 deletions(-) (limited to 'src/Wallabag/CoreBundle') diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php index d5820e66..dd9170ad 100644 --- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php @@ -7,6 +7,9 @@ use Psr\Log\LoggerInterface; use Wallabag\CoreBundle\Entity\Entry; use Wallabag\CoreBundle\Tools\Utils; use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeExtensionGuesser; +use Symfony\Component\Validator\Constraints\Language as LanguageConstraint; +use Symfony\Component\Validator\Constraints\Url as UrlConstraint; +use Symfony\Component\Validator\Validator\ValidatorInterface; /** * This kind of proxy class take care of getting the content from an url @@ -21,10 +24,11 @@ class ContentProxy protected $fetchingErrorMessage; protected $eventDispatcher; - public function __construct(Graby $graby, RuleBasedTagger $tagger, LoggerInterface $logger, $fetchingErrorMessage) + public function __construct(Graby $graby, RuleBasedTagger $tagger, ValidatorInterface $validator, LoggerInterface $logger, $fetchingErrorMessage) { $this->graby = $graby; $this->tagger = $tagger; + $this->validator = $validator; $this->logger = $logger; $this->mimeGuesser = new MimeTypeExtensionGuesser(); $this->fetchingErrorMessage = $fetchingErrorMessage; @@ -113,7 +117,24 @@ class ContentProxy $entry->setHeaders($content['all_headers']); } - $entry->setLanguage(isset($content['language']) ? $content['language'] : ''); + $this->validateAndSetLanguage( + $entry, + isset($content['language']) ? $content['language'] : '' + ); + + $this->validateAndSetPreviewPicture( + $entry, + isset($content['open_graph']['og_image']) ? $content['open_graph']['og_image'] : '' + ); + + // if content is an image define as a preview too + if (!empty($content['content_type']) && in_array($this->mimeGuesser->guess($content['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) { + $this->validateAndSetPreviewPicture( + $entry, + $content['url'] + ); + } + $entry->setMimetype(isset($content['content_type']) ? $content['content_type'] : ''); $entry->setReadingTime(Utils::getReadingTime($html)); @@ -122,15 +143,6 @@ class ContentProxy $entry->setDomainName($domainName); } - if (!empty($content['open_graph']['og_image'])) { - $entry->setPreviewPicture($content['open_graph']['og_image']); - } - - // if content is an image define as a preview too - if (!empty($content['content_type']) && in_array($this->mimeGuesser->guess($content['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) { - $entry->setPreviewPicture($content['url']); - } - try { $this->tagger->tag($entry); } catch (\Exception $e) { @@ -152,4 +164,48 @@ class ContentProxy { return !empty($content['title']) && !empty($content['html']) && !empty($content['url']); } + + /** + * Use a Symfony validator to ensure the language is well formatted. + * + * @param Entry $entry + * @param string $value Language to validate + */ + private function validateAndSetLanguage($entry, $value) + { + $errors = $this->validator->validate( + $value, + (new LanguageConstraint()) + ); + + if (0 === count($errors)) { + $entry->setLanguage($value); + + return; + } + + $this->logger->warning('Language validation failed. '.(string) $errors); + } + + /** + * Use a Symfony validator to ensure the preview picture is a real url. + * + * @param Entry $entry + * @param string $value URL to validate + */ + private function validateAndSetPreviewPicture($entry, $value) + { + $errors = $this->validator->validate( + $value, + (new UrlConstraint()) + ); + + if (0 === count($errors)) { + $entry->setPreviewPicture($value); + + return; + } + + $this->logger->warning('PreviewPicture validation failed. '.(string) $errors); + } } diff --git a/src/Wallabag/CoreBundle/Resources/config/services.yml b/src/Wallabag/CoreBundle/Resources/config/services.yml index a9b0d2d5..2ae5d27f 100644 --- a/src/Wallabag/CoreBundle/Resources/config/services.yml +++ b/src/Wallabag/CoreBundle/Resources/config/services.yml @@ -90,6 +90,7 @@ services: arguments: - "@wallabag_core.graby" - "@wallabag_core.rule_based_tagger" + - "@validator" - "@logger" - '%wallabag_core.fetching_error_message%' -- cgit v1.2.3 From be54dfe4e67b90bf6d94139fe968584871ca0f59 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 8 Jun 2017 21:56:20 +0200 Subject: CS --- src/Wallabag/CoreBundle/Helper/ContentProxy.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/Wallabag/CoreBundle') diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php index dd9170ad..f752d37e 100644 --- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php @@ -19,6 +19,7 @@ class ContentProxy { protected $graby; protected $tagger; + protected $validator; protected $logger; protected $mimeGuesser; protected $fetchingErrorMessage; @@ -127,7 +128,7 @@ class ContentProxy isset($content['open_graph']['og_image']) ? $content['open_graph']['og_image'] : '' ); - // if content is an image define as a preview too + // if content is an image, define it as a preview too if (!empty($content['content_type']) && in_array($this->mimeGuesser->guess($content['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) { $this->validateAndSetPreviewPicture( $entry, -- cgit v1.2.3 From 42f3bb2c6346e04d2837f980bf685f7e32a61a21 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Fri, 9 Jun 2017 11:28:04 +0200 Subject: Use Locale instead of Language --- src/Wallabag/CoreBundle/Helper/ContentProxy.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/Wallabag/CoreBundle') diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php index f752d37e..e4e7fb31 100644 --- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php @@ -7,7 +7,7 @@ use Psr\Log\LoggerInterface; use Wallabag\CoreBundle\Entity\Entry; use Wallabag\CoreBundle\Tools\Utils; use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeExtensionGuesser; -use Symfony\Component\Validator\Constraints\Language as LanguageConstraint; +use Symfony\Component\Validator\Constraints\Locale as LocaleConstraint; use Symfony\Component\Validator\Constraints\Url as UrlConstraint; use Symfony\Component\Validator\Validator\ValidatorInterface; @@ -176,7 +176,7 @@ class ContentProxy { $errors = $this->validator->validate( $value, - (new LanguageConstraint()) + (new LocaleConstraint()) ); if (0 === count($errors)) { -- cgit v1.2.3 From 80e49ba7b0320a5c4278c01f3d7851a9218e0919 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Fri, 9 Jun 2017 11:42:04 +0200 Subject: Convert - to _ in language Mostly to increase language supports --- src/Wallabag/CoreBundle/Helper/ContentProxy.php | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/Wallabag/CoreBundle') diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php index e4e7fb31..0c971863 100644 --- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php @@ -174,6 +174,10 @@ class ContentProxy */ private function validateAndSetLanguage($entry, $value) { + // some lang are defined as fr-FR, es-ES. + // replacing - by _ might increase language support + $value = str_replace('-', '_', $value); + $errors = $this->validator->validate( $value, (new LocaleConstraint()) -- cgit v1.2.3