Automattic / Automattic/jetpack
regex for parsing images from html has some flaws
- Dominant language
- PHP
- Stars
- 1.8k
- Forks
- 898
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 774
Description
Hi, I was looking through the regex for parsing images from the HTML recently, and noticed a couple flaws.
For reference, here's the regex:
`#(?:]+?href=["|\'](?P[^\s]+?)["|\'][^>]*?>\s*)?(?P]*?\s+?src=["|\'](?P[^\s]+?)["|\'].*?>){1}(?:\s*)?#is`
Notably, the character classes for quotation marks (single/double) have pipe characters in them. Normally, a pipe character indicates "alternation", but a character class already implies alternation and interprets a pipe character as a literal character instead. This means it would match an attribute surrounded by pipe characters, like so:
``
Of course, it isn't likely to ever encounter that pattern as it isn't valid HTML, but the | should be removed from all of the character classes.
Secondly, the HTML spec says that the equals sign for an attribute is followed or preceded by zero or more whitespace characters, so it would be good to account for those, even if they are not used very often.
The corrected regex would look like this:
`#(?:]+?href\s*=\s*["\'](?P[^\s]+?)["\'][^>]*?>\s*)?(?P]*?\s+?src\s*=\s*["\'](?P[^\s]+?)["\'].*?>){1}(?:\s*)?#is`
Contributor guide
Research direction
Search the Jetpack PHP codebase for the image-parsing regex quoted in the issue and identify its existing tests or usage. Check the parser's handling of quoted attributes and whitespace around equals signs, then verify that the relevant tests cover the corrected matching behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100