Skip to content

Commit

Permalink
Replace bleach with nh3 (#3696)
Browse files Browse the repository at this point in the history
Fixes #3693 

Aims to fully replace bleach with nh3 due to bleach deprecation.
Currently, [django-nh3](https://github.com/marksweb/django-nh3) is in
it's infancy, but seems like it could be an almost drop in replacement
for [django-bleach](https://github.com/marksweb/django-bleach), for I
[forked it](https://github.com/wes-otf/django-nh3) and made some small
additions that would allow it to work for our purposes and be smoothly
migrated.

Initial smoke testing in Hypha seems to work exactly as bleach did but
needs more extensive testing. Ideally I would smooth out some edges of
my fork and put in a PR to django-nh3. Let me know any
thoughts/questions!
  • Loading branch information
wes-otf committed May 8, 2024
1 parent c7b7433 commit bbd230f
Show file tree
Hide file tree
Showing 29 changed files with 149 additions and 78 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{% load i18n activity_tags bleach_tags markdown_tags submission_tags apply_tags heroicons %}
{% load i18n activity_tags nh3_tags markdown_tags submission_tags apply_tags heroicons %}

<div class="feed__item feed__item--{{ activity.type }} border shadow-sm rounded-sm pb-2 " id="communications#{{ activity.id }}">
<div class="feed__pre-content hidden lg:block">
Expand Down Expand Up @@ -48,7 +48,7 @@
data-visibility="{{activity.visibility}}"
data-edit-url="{% url 'api:v1:comments-edit' pk=activity.pk %}"
>
{{ activity|display_for:request.user|submission_links|markdown|bleach }}
{{ activity|display_for:request.user|submission_links|markdown|nh3 }}
</div>
<style>
@media only screen and (min-width: 1024px){
Expand All @@ -60,7 +60,7 @@
<div class="js-edit-block pe-3" aria-live="polite"></div>
{% else %}
<div class="px-3 prose">
{{ activity|display_for:request.user|submission_links|markdown|bleach }}
{{ activity|display_for:request.user|submission_links|markdown|nh3 }}
</div>
{% endif %}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{% load i18n activity_tags bleach_tags markdown_tags submission_tags apply_tags %}
{% load i18n activity_tags nh3_tags markdown_tags submission_tags apply_tags %}

<div class="notifications notifications--dropdown">
<div class="notifications__content zeta" role="activity">
Expand Down
3 changes: 1 addition & 2 deletions hypha/apply/activity/templates/activity/notifications.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{% extends "base-apply.html" %}
{% load i18n static activity_tags apply_tags bleach_tags markdown_tags submission_tags heroicons %}

{% load i18n static activity_tags apply_tags nh3_tags markdown_tags submission_tags heroicons %}
{% block content %}
<div class="admin-bar">
<div class="admin-bar__inner">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{% extends "messages/email/applicant_base.html" %}
{% load bleach_tags i18n %}
{% load nh3_tags i18n %}

{% block content %}{% trans "Your application has been reviewed and the outcome is" %}: {{ determination.clean_outcome }}

{{ determination.message|bleach|striptags }}
{{ determination.message|nh3|striptags }}

{% trans "Read the full determination here" %}: {{ request.scheme }}://{{ request.get_host }}{{ determination.get_absolute_url }}{% endblock %}
4 changes: 2 additions & 2 deletions hypha/apply/api/v1/serializers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from django.contrib.auth import get_user_model
from django_bleach.templatetags.bleach_tags import bleach_value
from django_nh3.templatetags.nh3_tags import nh3_value
from rest_framework import serializers

from hypha.apply.activity.models import Activity
Expand Down Expand Up @@ -416,7 +416,7 @@ class Meta:
)

def get_message(self, obj):
return bleach_value(markdown_to_html(obj.message))
return nh3_value(markdown_to_html(obj.message))

def get_editable(self, obj):
return self.context["request"].user == obj.user
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% extends "base-apply.html" %}
{% load render_table from django_tables2 %}
{% load i18n static wagtailcore_tags workflow_tags statusbar_tags heroicons dashboard_statusbar_tags apply_tags invoice_tools markdown_tags bleach_tags %}
{% load i18n static wagtailcore_tags workflow_tags statusbar_tags heroicons dashboard_statusbar_tags apply_tags invoice_tools markdown_tags nh3_tags %}
{% block body_class %}bg-light-grey{% endblock %}

{% block title %}{% trans "Dashboard" %}{% endblock %}
Expand Down Expand Up @@ -31,7 +31,7 @@ <h2 class="text-center font-light">{% trans "My tasks" %}</h2>
{% for task in my_tasks.data %}
<div class="bg-white p-1 flex mb-1 items-center">
<svg class="icon icon--dashboard-tasks"><use xlink:href="#{{ task.icon }}"></use></svg>
<div class="flex-1">{{ task.text|markdown|bleach }}</div>
<div class="flex-1">{{ task.text|markdown|nh3 }}</div>
<a class="button button-primary m-2" href="{{ task.url }}">View</a>
</div>
{% endfor %}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% extends "base-apply.html" %}
{% load render_table from django_tables2 %}
{% load i18n static markdown_tags bleach_tags %}
{% load i18n static markdown_tags nh3_tags %}

{% block title %}{% trans "Dashboard" %}{% endblock %}

Expand All @@ -21,7 +21,7 @@ <h2 class="text-center font-light">{% trans "My tasks" %}</h2>
{% for task in my_tasks.data %}
<div class="bg-white p-1 flex mb-1 items-center">
<svg class="icon icon--dashboard-tasks"><use xlink:href="#{{ task.icon }}"></use></svg>
<div class="flex-1">{{ task.text|markdown|bleach }}</div>
<div class="flex-1">{{ task.text|markdown|nh3 }}</div>
<a class="button button-primary m-2" href="{{ task.url }}">View</a>
</div>
{% endfor %}
Expand Down
4 changes: 2 additions & 2 deletions hypha/apply/dashboard/templates/dashboard/dashboard.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% extends "base-apply.html" %}
{% load render_table from django_tables2 %}
{% load i18n static bleach_tags markdown_tags %}
{% load i18n static nh3_tags markdown_tags %}

{% block extra_css %}
{{ my_reviewed.filterset.form.media.css }}
Expand Down Expand Up @@ -30,7 +30,7 @@ <h2 class="text-center font-light">{% trans "My tasks" %}</h2>
{% for task in my_tasks.data %}
<div class="bg-white p-1 flex mb-1 items-center">
<svg class="icon icon--dashboard-tasks"><use xlink:href="#{{ task.icon }}"></use></svg>
<div class="flex-1">{{ task.text|markdown|bleach }}</div>
<div class="flex-1">{{ task.text|markdown|nh3 }}</div>
<a class="button button-primary m-2" href="{{ task.url }}">View</a>
</div>
{% endfor %}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% extends "base-apply.html" %}
{% load render_table from django_tables2 %}
{% load i18n static markdown_tags bleach_tags %}
{% load i18n static markdown_tags nh3_tags %}

{% block title %}{% trans "Dashboard" %}{% endblock %}

Expand All @@ -21,7 +21,7 @@ <h2 class="text-center font-light">{% trans "My tasks" %}</h2>
{% for task in my_tasks.data %}
<div class="bg-white p-1 flex mb-1 items-center">
<svg class="icon icon--dashboard-tasks"><use xlink:href="#{{ task.icon }}"></use></svg>
<div class="flex-1">{{ task.text|markdown|bleach }}</div>
<div class="flex-1">{{ task.text|markdown|nh3 }}</div>
<a class="button button-primary m-2" href="{{ task.url }}">View</a>
</div>
{% endfor %}
Expand Down
4 changes: 2 additions & 2 deletions hypha/apply/determinations/models.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import bleach
import nh3
from django.conf import settings
from django.core.serializers.json import DjangoJSONEncoder
from django.db import models
Expand Down Expand Up @@ -130,7 +130,7 @@ class Determination(DeterminationFormFieldsMixin, AccessFormData, models.Model):

@property
def stripped_message(self):
return bleach.clean(self.message, tags=[], strip=True)
return nh3.clean(self.message, tags=set())

@property
def clean_outcome(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% extends "base-apply.html" %}
{% load i18n static bleach_tags %}
{% load i18n static nh3_tags %}
{% block title %}{% if object %}{% trans "Edit a Determination" %} {% if object.is_draft %}{% trans "draft" %}{% endif %}{% else %}{% trans "Create a Determination" %}{% endif %}{% endblock %}
{% block content %}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% extends "base-apply.html" %}
{% load i18n bleach_tags heroicons %}
{% load i18n nh3_tags heroicons %}

{% block title %}{% trans "Determination for" %} {{ determination.submission.title }}{% endblock %}

Expand Down Expand Up @@ -32,14 +32,14 @@ <h5 class="determination-outcome mb-4">

<div class="rich-text rich-text--answers prose">
<h4>{% trans "Determination message" %}</h4>
{{ determination.message|bleach }}
{{ determination.message|nh3 }}
{% for group in determination.detailed_data.values %}
{% if group.title %}
<h4>{{ group.title|bleach }}</h4>
<h4>{{ group.title|nh3 }}</h4>
{% endif %}
{% for question, answer in group.questions %}
<h5>{{ question }}</h5>
{% if answer %}{% if answer == True %}{{ answer|yesno:"Agree,Disagree" }}{% else %}{{ answer|bleach }}{% endif %}{% else %}-{% endif %}
{% if answer %}{% if answer == True %}{{ answer|yesno:"Agree,Disagree" }}{% else %}{{ answer|nh3 }}{% endif %}{% else %}-{% endif %}
{% endfor %}
{% endfor %}
</div>
Expand Down
26 changes: 20 additions & 6 deletions hypha/apply/funds/differ.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import re
from difflib import SequenceMatcher
from typing import Tuple

from bleach.sanitizer import Cleaner
import nh3
from django.utils.html import format_html
from django.utils.safestring import mark_safe

Expand All @@ -16,13 +17,26 @@ def wrap_added(text):
return format_html('<span class="bg-green-200">{}</span>', mark_safe(text))


def compare(answer_a, answer_b, should_bleach=True):
if should_bleach:
cleaner = Cleaner(tags=["h4"], attributes={}, strip=True)
def compare(answer_a: str, answer_b: str, should_clean: bool = True) -> Tuple[str, str]:
"""Compare two strings, populate diff HTML and insert it, and return a tuple of the given strings.
Args:
answer_a:
The original string
answer_b:
The string to compare to the original
should_clean:
Optional boolean to determine if the string should be sanitized with NH3 (default=True)
Returns:
A tuple of the original strings with diff HTML inserted.
"""

if should_clean:
answer_a = re.sub("(<li[^>]*>)", r"\1◦ ", answer_a)
answer_b = re.sub("(<li[^>]*>)", r"\1◦ ", answer_b)
answer_a = cleaner.clean(answer_a)
answer_b = cleaner.clean(answer_b)
answer_a = nh3.clean(answer_a, tags={"h4"}, attributes={})
answer_b = nh3.clean(answer_b, tags={"h4"}, attributes={})

diff = SequenceMatcher(None, answer_a, answer_b)
from_diff = []
Expand Down
4 changes: 2 additions & 2 deletions hypha/apply/funds/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from itertools import groupby
from operator import methodcaller

import bleach
import nh3
from django import forms
from django.db.models import Q
from django.utils.safestring import mark_safe
Expand Down Expand Up @@ -448,7 +448,7 @@ def make_role_reviewer_fields():
staff_reviewers = User.objects.staff().only("full_name", "pk")

for role in ReviewerRole.objects.all().order_by("order"):
role_name = bleach.clean(role.name, strip=True)
role_name = nh3.clean(role.name, tags=set())
field_name = f"role_reviewer_{role.id}"
field = forms.ModelChoiceField(
queryset=staff_reviewers,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% extends "base-apply.html" %}
{% load i18n static bleach_tags heroicons %}
{% load i18n static nh3_tags heroicons %}

{% block title %}{% trans "Report" %} | {{ object.project.title }}{% endblock %}
{% block body_class %}{% endblock %}
Expand Down Expand Up @@ -28,12 +28,12 @@ <h2>{% trans "Report Skipped" %}</h2>
{% else %}
<h4>{% trans "Public Report" %}</h4>
<div class="rich-text">
{{ object.current.public_content|bleach|safe }}
{{ object.current.public_content|nh3|safe }}
</div>

<h4>{% trans "Private Report" %}</h4>
<div class="rich-text">
{{ object.current.private_content|bleach|safe }}
{{ object.current.private_content|nh3|safe }}
</div>
{% for file in object.current.files.all %}
{% if forloop.first %}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% extends "base-apply.html" %}
{% load bleach_tags i18n approval_tools heroicons %}
{% load nh3_tags i18n approval_tools heroicons %}
{% user_can_edit_project object request.user as editable %}
{% block title %}{% trans "Contracting Information for" %} {{ project.title }} {% endblock %}

Expand Down Expand Up @@ -31,7 +31,7 @@ <h5 class="vendor-info">{% trans "Last Updated" %}: {{ vendor.updated_at|date:'D
<div class="rich-text rich-text--answers">
{% for group in vendor_detailed_response.values %}
{% if group.title %}
<h1>{{ group.title|bleach }}</h4>
<h1>{{ group.title|nh3 }}</h4>
{% endif %}
{% for question, answer in group.questions %}
<h5>{{ question }}</h5>
Expand All @@ -44,7 +44,7 @@ <h5>{{ question }}</h5>
</div>
</div>
{% else %}
<p>{% if answer == True or answer == False %}{{ answer|yesno:"Yes,No" }}{% else %}{% if answer %}{{ answer|bleach }}{% else %}-{% endif %}{% endif %}</p>
<p>{% if answer == True or answer == False %}{{ answer|yesno:"Yes,No" }}{% else %}{% if answer %}{{ answer|nh3 }}{% else %}-{% endif %}{% endif %}</p>
{% endif %}
{% endfor %}
{% endfor %}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{% load bleach_tags %}
{% load nh3_tags %}
{% block data_display %}
<div>
<h5>{{ value.field_label }}</h5>
<div>{{ score }}</div>
<div>{{ comment|bleach }}</div>
<div>{{ comment|nh3 }}</div>
</div>
{% endblock %}
2 changes: 1 addition & 1 deletion hypha/apply/review/templates/review/review_detail.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% extends "base-apply.html" %}
{% load i18n bleach_tags submission_tags heroicons %}
{% load i18n nh3_tags submission_tags heroicons %}
{% block title %}{% trans "Review for" %} {{ review.submission.title }}{% endblock %}
{% block content %}

Expand Down
4 changes: 2 additions & 2 deletions hypha/apply/review/templates/review/review_list.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% extends "base-apply.html" %}
{% load i18n bleach_tags review_tags workflow_tags %}
{% load i18n nh3_tags review_tags workflow_tags %}

{% block title %}{% trans "Reviews" %}{% endblock %}

Expand Down Expand Up @@ -28,7 +28,7 @@
{% elif answers.question == "Opinions"%}
<td class="reviews-list__td">{{ answer }}</td>
{% else %}
<td class="reviews-list__td">{{ answer|bleach }}</td>
<td class="reviews-list__td">{{ answer|nh3 }}</td>
{% endif %}
{% endfor %}
</tr>
Expand Down
10 changes: 5 additions & 5 deletions hypha/apply/stream_forms/blocks.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Credit to https://github.com/BertrandBordage for initial implementation
import bleach
import nh3
from anyascii import anyascii
from dateutil.parser import isoparse, parse
from django import forms
Expand All @@ -11,7 +11,7 @@
from django.utils.html import conditional_escape
from django.utils.text import slugify
from django.utils.translation import gettext_lazy as _
from django_bleach.templatetags.bleach_tags import bleach_value
from django_nh3.templatetags.nh3_tags import nh3_value
from wagtail.blocks import (
BooleanBlock,
CharBlock,
Expand Down Expand Up @@ -89,7 +89,7 @@ def serialize_no_response(self, value, context):
}

def prepare_data(self, value, data, serialize=False):
return bleach_value(str(data))
return nh3_value(str(data))

def render(self, value, context):
data = context.get("data")
Expand Down Expand Up @@ -141,7 +141,7 @@ def get_field_class(self, struct_value):
def get_searchable_content(self, value, data):
# CharField acts as a fallback. Force data to string
data = str(data)
return bleach.clean(data or "", tags=[], strip=True)
return nh3.clean(data or "", tags=set())


class MultiInputCharFieldBlock(CharFieldBlock):
Expand All @@ -165,7 +165,7 @@ class Meta:
template = "stream_forms/render_unsafe_field.html"

def get_searchable_content(self, value, data):
return bleach.clean(data or "", tags=[], strip=True)
return nh3.clean(data or "", tags=set())


class NumberFieldBlock(OptionalFormFieldBlock):
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{% extends "stream_forms/render_field.html" %}
{% load bleach_tags markdown_tags %}
{% load nh3_tags markdown_tags %}
{% block data_display %}
{% if data %}
{{ data|markdown|bleach }}
{{ data|markdown|nh3 }}
{% else %}
{{ block.super }}
{% endif %}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{% extends "stream_forms/render_field.html" %}
{% load bleach_tags %}
{% load nh3_tags %}
{% block data_display %}
{% if data %}
{% if value.format == 'url' %}
<a class="link" href="{{ data|bleach }}" target="_blank" rel="noopener noreferrer">{{ data|bleach }}</a>
<a class="link" href="{{ data|nh3 }}" target="_blank" rel="noopener noreferrer">{{ data|nh3 }}</a>
{% elif value.format == 'email' %}
<a class="u-email" href="mailto:{{ data|bleach }}">{{ data|bleach }}</a>
<a class="u-email" href="mailto:{{ data|nh3 }}">{{ data|nh3 }}</a>
{% else %}
<p>{{ data|bleach }}</p>
<p>{{ data|nh3 }}</p>
{% endif %}
{% else %}
{{ block.super }}
Expand Down
Loading

0 comments on commit bbd230f

Please sign in to comment.