Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix HTML elements not unhashing correctly (issue 508) #509

Merged
merged 3 commits into from Apr 28, 2023

Conversation

Crozzers
Copy link
Contributor

This PR fixes #508 by patching the close tag detection in the strict block tag hashing function.

The issue was with lines such as:

<div><div></div>
</div>

Where the first opening <div> tag would be deemed as "closed" because there was a closing </div> tag present on the same line.

The fix is to count the number of opening and closing tags on a single line. If they are inequal, the tag is deemed "open".

@Crozzers Crozzers changed the title Fix issue 508 Fix HTML elements not unhashing correctly (issue 508) Apr 26, 2023
@@ -908,6 +908,10 @@ def _strict_tag_block_sub(self, text, html_tags_re, callback):

return result

def _tag_is_closed(self, tag_name, text):
# super basic check if number of open tags == number of closing tags
return len(re.findall('<%s(?:.*?)>' % tag_name, text)) == len(re.findall('</%s>' % tag_name, text))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if this could be even simpler on the left side, something as simple as even:

len(re.findall('<%s' % tag_name, text)) == len(re.findall('</%s>' % tag_name, text))

which just bails on matching the opening element's end > and middle content.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mainly put that there because I was concerned about incomplete tags potentially messing it up. Not sure as to how something like <div> <div </div> should be interpreted

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup good call

@nicholasserra
Copy link
Collaborator

Thank you!

@nicholasserra nicholasserra merged commit 4cf766c into trentm:master Apr 28, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTML elements are replaced by hashes
2 participants