Revert "chore: Use nh3 lib instead of bleach (#23862)"

This reverts commit 689bc8e.
apache · Jan 12, 2024 · 9cedc15 · 9cedc15
1 parent 3de71e2
commit 9cedc15
Show file tree

Hide file tree

Showing 8 changed files with 36 additions and 38 deletions.
diff --git a/requirements/base.txt b/requirements/base.txt
@@ -30,12 +30,15 @@ bcrypt==4.0.1
     # via paramiko
 billiard==3.6.4.0
     # via celery
+bleach==3.3.1
+    # via apache-superset
 bottleneck==1.3.7
     # via pandas
 brotli==1.0.9
     # via flask-compress
 cachelib==0.9.0
     # via
+    #   apache-superset
     #   flask-caching
     #   flask-session
 cattrs==23.2.1
@@ -143,9 +146,7 @@ geographiclib==1.52
 geopy==2.2.0
     # via apache-superset
 greenlet==2.0.2
-    # via
-    #   shillelagh
-    #   sqlalchemy
+    # via shillelagh
 gunicorn==21.2.0
     # via apache-superset
 hashids==1.3.1
@@ -211,8 +212,6 @@ mdurl==0.1.2
     # via markdown-it-py
 msgpack==1.0.2
     # via apache-superset
-nh3==0.2.11
-    # via apache-superset
 numba==0.57.1
     # via pandas
 numexpr==2.8.4
@@ -231,6 +230,7 @@ packaging==23.1
     # via
     #   apache-superset
     #   apispec
+    #   bleach
     #   deprecation
     #   gunicorn
     #   limits
@@ -316,6 +316,7 @@ simplejson==3.17.3
     # via apache-superset
 six==1.16.0
     # via
+    #   bleach
     #   click-repl
     #   isodate
     #   paramiko
@@ -367,6 +368,8 @@ vine==5.0.0
     #   kombu
 wcwidth==0.2.5
     # via prompt-toolkit
+webencodings==0.5.1
+    # via bleach
 werkzeug==2.3.3
     # via
     #   apache-superset

diff --git a/requirements/development.txt b/requirements/development.txt
@@ -72,7 +72,7 @@ pexpect==4.8.0
     # via ipython
 pickleshare==0.7.5
     # via ipython
-pillow==9.5.0
+pillow==10.2.0
     # via apache-superset
 progress==1.6
     # via -r requirements/development.in

diff --git a/setup.cfg b/setup.cfg
@@ -30,7 +30,7 @@ combine_as_imports = true
 include_trailing_comma = true
 line_length = 88
 known_first_party = superset
-known_third_party =alembic,apispec,backoff,celery,click,colorama,cron_descriptor,croniter,cryptography,dateutil,deprecation,flask,flask_appbuilder,flask_babel,flask_caching,flask_compress,flask_jwt_extended,flask_login,flask_migrate,flask_sqlalchemy,flask_talisman,flask_testing,flask_wtf,freezegun,geohash,geopy,holidays,humanize,isodate,jinja2,jwt,markdown,markupsafe,marshmallow,msgpack,nh3,numpy,pandas,parameterized,parsedatetime,pgsanity,pkg_resources,polyline,prison,progress,pyarrow,sqlalchemy_bigquery,pyhive,pyparsing,pytest,pytest_mock,pytz,redis,requests,selenium,setuptools,shillelagh,simplejson,slack,sqlalchemy,sqlalchemy_utils,sqlparse,typing_extensions,urllib3,werkzeug,wtforms,wtforms_json,yaml
+known_third_party =alembic,apispec,backoff,celery,click,colorama,cron_descriptor,croniter,cryptography,dateutil,deprecation,flask,flask_appbuilder,flask_babel,flask_caching,flask_compress,flask_jwt_extended,flask_login,flask_migrate,flask_sqlalchemy,flask_talisman,flask_testing,flask_wtf,freezegun,geohash,geopy,holidays,humanize,isodate,jinja2,jwt,markdown,markupsafe,marshmallow,msgpack,bleach,numpy,pandas,parameterized,parsedatetime,pgsanity,pkg_resources,polyline,prison,progress,pyarrow,sqlalchemy_bigquery,pyhive,pyparsing,pytest,pytest_mock,pytz,redis,requests,selenium,setuptools,simplejson,slack,sqlalchemy,sqlalchemy_utils,sqlparse,typing_extensions,urllib3,werkzeug,wtforms,wtforms_json,yaml
 multi_line_output = 3
 order_by_type = false
 

diff --git a/setup.py b/setup.py
@@ -74,6 +74,8 @@ def get_git_sha() -> str:
     },
     install_requires=[
         "backoff>=1.8.0",
+        "bleach>=3.0.2, <4.0.0",
+        "cachelib>=0.9.0,<0.10.0",
         "celery>=5.2.2, <6.0.0",
         "click>=8.0.3",
         "click-option-group",
@@ -102,7 +104,6 @@ def get_git_sha() -> str:
         "Mako>=1.2.2",
         "markdown>=3.0",
         "msgpack>=1.0.0, <1.1",
-        "nh3>=0.2.11, <0.3",
         "numpy==1.23.5",
         "packaging",
         "pandas[performance]>=2.0.3, <2.1",

diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
@@ -475,10 +475,10 @@ class Chart extends React.Component {
 
         {/*
           This usage of dangerouslySetInnerHTML is safe since it is being used to render
-          markdown that is sanitized with nh3. See:
+          markdown that is sanitized with bleach. See:
              https://github.com/apache/superset/pull/4390
           and
-             https://github.com/apache/superset/pull/23862
+             https://github.com/apache/superset/commit/b6fcc22d5a2cb7a5e92599ed5795a0169385a825
         */}
         {isExpanded && slice.description_markeddown && (
           <div

diff --git a/superset/reports/notifications/email.py b/superset/reports/notifications/email.py
@@ -21,7 +21,7 @@
 from email.utils import make_msgid, parseaddr
 from typing import Any, Optional
 
-import nh3
+import bleach
 from flask_babel import gettext as __
 
 from superset import app
@@ -34,10 +34,10 @@
 
 logger = logging.getLogger(__name__)
 
-TABLE_TAGS = {"table", "th", "tr", "td", "thead", "tbody", "tfoot"}
-TABLE_ATTRIBUTES = {"colspan", "rowspan", "halign", "border", "class"}
+TABLE_TAGS = ["table", "th", "tr", "td", "thead", "tbody", "tfoot"]
+TABLE_ATTRIBUTES = ["colspan", "rowspan", "halign", "border", "class"]
 
-ALLOWED_TAGS = {
+ALLOWED_TAGS = [
     "a",
     "abbr",
     "acronym",
@@ -53,14 +53,13 @@
     "p",
     "strong",
     "ul",
-}.union(TABLE_TAGS)
+] + TABLE_TAGS
 
-ALLOWED_TABLE_ATTRIBUTES = {tag: TABLE_ATTRIBUTES for tag in TABLE_TAGS}
 ALLOWED_ATTRIBUTES = {
-    "a": {"href", "title"},
-    "abbr": {"title"},
-    "acronym": {"title"},
-    **ALLOWED_TABLE_ATTRIBUTES,
+    "a": ["href", "title"],
+    "abbr": ["title"],
+    "acronym": ["title"],
+    **{tag: TABLE_ATTRIBUTES for tag in TABLE_TAGS},
 }
 
 
@@ -108,8 +107,7 @@ def _get_content(self) -> EmailContent:
             }
 
         # Strip any malicious HTML from the description
-        # pylint: disable=no-member
-        description = nh3.clean(
+        description = bleach.clean(
             self._content.description or "",
             tags=ALLOWED_TAGS,
             attributes=ALLOWED_ATTRIBUTES,
@@ -118,13 +116,12 @@ def _get_content(self) -> EmailContent:
         # Strip malicious HTML from embedded data, allowing only table elements
         if self._content.embedded_data is not None:
             df = self._content.embedded_data
-            # pylint: disable=no-member
-            html_table = nh3.clean(
+            html_table = bleach.clean(
                 df.to_html(na_rep="", index=True, escape=True),
                 # pandas will escape the HTML in cells already, so passing
                 # more allowed tags here will not work
                 tags=TABLE_TAGS,
-                attributes=ALLOWED_TABLE_ATTRIBUTES,
+                attributes=TABLE_ATTRIBUTES,
             )
         else:
             html_table = ""

diff --git a/superset/utils/core.py b/superset/utils/core.py
@@ -49,12 +49,13 @@
 from io import BytesIO
 from timeit import default_timer
 from types import TracebackType
-from typing import Any, Callable, cast, NamedTuple, TYPE_CHECKING, TypedDict, TypeVar
+from typing import Any, Callable, cast, NamedTuple, TYPE_CHECKING, TypeVar, Optional, \
+    TypedDict
 from urllib.parse import unquote_plus
 from zipfile import ZipFile
 
+import bleach
 import markdown as md
-import nh3
 import numpy as np
 import pandas as pd
 import sqlalchemy as sa
@@ -566,8 +567,8 @@ def error_msg_from_exception(ex: Exception) -> str:
     return msg or str(ex)
 
 
-def markdown(raw: str, markup_wrap: bool | None = False) -> str:
-    safe_markdown_tags = {
+def markdown(raw: str, markup_wrap: Optional[bool] = False) -> str:
+    safe_markdown_tags = [
         "h1",
         "h2",
         "h3",
@@ -593,10 +594,10 @@ def markdown(raw: str, markup_wrap: bool | None = False) -> str:
         "dt",
         "img",
         "a",
-    }
+    ]
     safe_markdown_attrs = {
-        "img": {"src", "alt", "title"},
-        "a": {"href", "alt", "title"},
+        "img": ["src", "alt", "title"],
+        "a": ["href", "alt", "title"],
     }
     safe = md.markdown(
         raw or "",
@@ -606,8 +607,7 @@ def markdown(raw: str, markup_wrap: bool | None = False) -> str:
             "markdown.extensions.codehilite",
         ],
     )
-    # pylint: disable=no-member
-    safe = nh3.clean(safe, tags=safe_markdown_tags, attributes=safe_markdown_attrs)
+    safe = bleach.clean(safe, safe_markdown_tags, safe_markdown_attrs)
     if markup_wrap:
         safe = Markup(safe)
     return safe

diff --git a/tests/unit_tests/notifications/email_tests.py b/tests/unit_tests/notifications/email_tests.py
@@ -50,8 +50,5 @@ def test_render_description_with_html() -> None:
         ._get_content()
         .body
     )
-    assert (
-        '<p>This is <a href="#" rel="noopener noreferrer">a test</a> alert</p><br>'
-        in email_body
-    )
+    assert '<p>This is <a href="#">a test</a> alert</p><br>' in email_body
     assert '<td>&lt;a href="http://www.example.com"&gt;333&lt;/a&gt;</td>' in email_body