Add option to detect malformed byte sequences #200
Conversation
There was a problem hiding this comment.
I think the mb_check_encoding() will detect any invalid byte sequence, so the language used should move from "malformed" (which is a specific issue) to "invalid" (which covers all detected issues).
I've provided change suggestions for the code, but the tests will also needed to be adapted to match the adapted code.
src/Dom/Document.php
Outdated
| private function assertNoneMalformedByteSequences($source) | ||
| { | ||
| if ($this->options[Option::ADDITIONAL_VALIDATION] && ! mb_check_encoding($source)) { | ||
| throw new \InvalidArgumentException( |
There was a problem hiding this comment.
This should use a custom exception InvalidByteSequence with a named constructor (see src/Exception).
There was a problem hiding this comment.
Should the exception message contain an HTML source?
If not, is there necessary to write a test for InvalidByteSequence that contains only a message?
There was a problem hiding this comment.
Normally yes, but for string-based HTML, it is actually hard to produce something useful for an exception message that doesn't flood the screen/logs... Not sure how we would go about that here.
There was a problem hiding this comment.
The third way is to add a second argument to the named constructor and then provide a method like $invalidByteSequence->getHtmlSource() to get the html contained invalid byte sequence.
Co-authored-by: Alain Schlesser <alain.schlesser@gmail.com>
|
I'm with you on that, "invalid" is a good choice for Thank you for your detailed review. I appreciate it. |
|
Thanks for the PR, @06romix ! |
This PR adds the option for detecting malformed byte sequences according to #24