Skip to content

feat: consolidate HTML sanitizing#8471

Merged
rjsparks merged 12 commits into
ietf-tools:mainfrom
jennifer-richards:bleach
Jan 28, 2025
Merged

feat: consolidate HTML sanitizing#8471
rjsparks merged 12 commits into
ietf-tools:mainfrom
jennifer-richards:bleach

Conversation

@jennifer-richards

Copy link
Copy Markdown
Member

This drops lxml.html.clean.Cleaner, which was used for sanitizing uploaded HTML, instead using bleach for all sanitizing. Refactors the organization of bleach settings, moving all the cleaning into ietf.utils.html and leaving only linkify-related settings in ietf.utils.text.

This refactor drops code that added a <meta charset="utf-8"> to the <head> when the latter was present in a cleaned upload. It probably also did away with some incidental cleanup that lxml was doing for us under some circumstances. One example I came across was that a test clean of

<html><body>Text in the body element</body></html>

became

<html><body><p>Text in the body element</p></body></html>

This only happened in some code paths and was neither tested nor documented, so I don't think it's a big loss. If we need that level of cleanup, we should add it as a proper feature.

@jennifer-richards

Copy link
Copy Markdown
Member Author

Tagging @NGPixel @holloway since we talked about this

NGPixel
NGPixel previously approved these changes Jan 27, 2025
@NGPixel

NGPixel commented Jan 27, 2025

Copy link
Copy Markdown
Member

Python code looks pythony enough

@codecov

codecov Bot commented Jan 28, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 97.22222% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.80%. Comparing base (a4eeae5) to head (fe74aa3).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
ietf/utils/html.py 91.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8471   +/-   ##
=======================================
  Coverage   88.80%   88.80%           
=======================================
  Files         312      312           
  Lines       40886    40856   -30     
=======================================
- Hits        36308    36284   -24     
+ Misses       4578     4572    -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rjsparks
rjsparks merged commit e91bda7 into ietf-tools:main Jan 28, 2025
@jennifer-richards
jennifer-richards deleted the bleach branch January 29, 2025 15:09
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Feb 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants