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

The > sign should not be escaped within conditional CSS #11804

Closed
felipemelendez opened this issue Feb 1, 2023 · 0 comments · Fixed by #11805
Closed

The > sign should not be escaped within conditional CSS #11804

felipemelendez opened this issue Feb 1, 2023 · 0 comments · Fixed by #11805
Labels

Comments

@felipemelendez
Copy link
Contributor

Email templates for MS Outlook require conditional CSS. A line of code for that looks like this: <!--[if (gte mso 9)|(IE)]>
That is an if block that will be called if the version of MSOutlook receiving the email is greater than 9 or if it’s any version of Internet Explorer.

Emails sent to MSO users are displaying the code for the conditional CSS:
Screenshot 2023-02-01 at 1 24 43 PM

If you look closely you’ll see that the line of code that I used as an example looks like this: <!--[if (gte mso 9)|(IE)]&gt;
The transformation of > into it's HTML counterpart, &gt; is not allowing the if block to adequately close.

The source of that transformation comes from: twisted.web._flatten.escapedComment where this line of code exists: data = data.replace(b"--", b"- - ").replace(b">", b"&gt;")

That was introduced in this PR

In the docstring of test_commentEscaping this statement was made: "Comments start with and never contain -- or >." That is not correct given that we close if statements with a >
i.e. <!--[if (gte mso 9)|(IE)]>

Also, the referenced XML spec defines comment as:

Char	   ::=   	#x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]	/* any Unicode character, excluding the surrogate blocks, FFFE, and FFFF. */
Comment	   ::=   	'<!--' ((Char - '-') | ('-' (Char - '-')))* '-->'

And > would be #x3E.

To solve this issue we patched twisted.web._flatten.escapeComment at runtime, removing .replace(b">", b"&gt;")

I am creating this issue to introduce the problem and subsequently I will submit a PR that aims to solve the problem.

Testing environment

  • Python version 3.10.4
  • Twisted version 22.10.0
  • ProductName: macOS
    ProductVersion: 13.1
    BuildVersion: 22C65

$ pip freeze
alabaster==0.7.13
appdirs==1.4.4
astor==0.8.1
astroid==2.14.1
attrs==22.2.0
Automat==22.10.0
Babel==2.11.0
CacheControl==0.12.11
cachetools==5.3.0
certifi==2022.12.7
cfgv==3.3.1
chardet==5.1.0
charset-normalizer==3.0.1
click==8.1.3
click-default-group==1.2.2
colorama==0.4.6
ConfigArgParse==1.5.3
constantly==15.1.0
coverage==6.6.0b1
dill==0.3.6
distlib==0.3.6
docutils==0.19
exceptiongroup==1.1.0
filelock==3.9.0
hyperlink==21.0.0
hypothesis==6.65.2
identify==2.5.17
idna==3.4
imagesize==1.4.1
incremental==22.10.0
isort==5.12.0
Jinja2==3.1.2
lazy-object-proxy==1.9.0
lockfile==0.12.2
lunr==0.6.2
MarkupSafe==2.1.2
mccabe==0.7.0
msgpack==1.0.4
nodeenv==1.7.0
packaging==23.0
platformdirs==2.6.2
pluggy==1.0.0
pre-commit==3.0.2
pydoctor==22.9.1
pyflakes==2.5.0
Pygments==2.14.0
PyHamcrest==2.0.4
pylint==2.16.0
pyproject_api==1.5.0
pytz==2022.7.1
PyYAML==6.0
readthedocs-sphinx-ext==2.2.0
requests==2.28.2
six==1.16.0
snowballstemmer==2.2.0
sortedcontainers==2.4.0
Sphinx==5.3.0
sphinx-rtd-theme==0.5.1
sphinxcontrib-applehelp==1.0.4
sphinxcontrib-devhelp==1.0.2
sphinxcontrib-htmlhelp==2.0.1
sphinxcontrib-jsmath==1.0.1
sphinxcontrib-qthelp==1.0.3
sphinxcontrib-serializinghtml==1.1.5
toml==0.10.2
tomli==2.0.1
tomlkit==0.11.6
towncrier==22.12.0
tox==4.4.4
-e git+https://github.com/twisted/twisted.git@59783c7c2e40868283395b2a88125618b254631c#egg=Twisted
twistedchecker==0.7.4
typing_extensions==4.4.0
urllib3==1.26.14
virtualenv==20.17.1
wrapt==1.14.1
zope.interface==5.5.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant