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

full CSS is escaped if a style definition seems invalid #117

Closed
optyler opened this issue Dec 10, 2021 · 4 comments
Closed

full CSS is escaped if a style definition seems invalid #117

optyler opened this issue Dec 10, 2021 · 4 comments

Comments

@optyler
Copy link

optyler commented Dec 10, 2021

Hi guys.

I don't know if this is a bug or just a lack of knowledge about the options... so, sorry if i'm in the wrong place.

I'm writing for my company a newsletter template that is filled with documents and render on an admin webpage. Administrators can then edit the pre-rendered newsletter and edit it, if they want, in a richtext box, before sending to the customers. So, when they save their changes, we POST the full modified html, check it with antisamy, and save it in DB.

If you ever worked to produce newsletters, you know we have to put tons of css for each kind of mail clients, use specific vendor prefixes, all sort of media queries and so on...

Here is the content of a style tag inserted in the head of my document.

<style type="text/css">
.ReadMsgBody {
  width: 100%;
}
p {
  margin: 10px 0;
  padding: 0px;
}
table {
  border-collapse: collapse;
}
p, a, li, td, blockquote {
   mso-line-height-rule: exactly; /* PAY ATTENTION TO THIS LINE*/
}
h1 {
color: #2C445A;
font-family: Verdana, Geneva, sans-serif;
font-size: 15px;
font-style: normal;
font-weight: bold;
line-height: 150%;
letter-spacing: normal;
text-align: center;
}
h2 {
font-family: Verdana, Geneva, sans-serif;
color: #fff;
font-size: 15px;
font-weight: bold;
}
[...] and much more

After validation, the CSS was truncated after the first "non standard" property.
Here is the result :

<style type="text/css">
*.ReadMsgBody {
	width: 100.0%;
}
p {
	margin: 10.0px 0;
	padding: 0.0px;
}
table {
	border-collapse: collapse;
}
p, a, li, td, blockquote {
}
	</style>

All the content after mso-line-height-rule: exactly; has disappeared.

Here are my questions :

  • is this a bug ?
  • is there an option to "allow vendor prefixes" ?
  • is there an option to "continue the validation if a property is invalide" ?
  • is there a way to validate media queries ? ( @media only screen and (max-width: 480px) { )

Thanks

@davewichers
Copy link
Collaborator

@spassarop - Hey - this question is for you :-)

@spassarop
Copy link
Collaborator

Hi @optyler, I'll answser by parts:

is this a bug ?

Partially, explained next.

is there an option to "allow vendor prefixes" ?

Kind of. You cannot provide a prefix itself and there is no separate option. The supported solution for this is to add the whole attribute to the policy. Here is an example to support mso-line-height-rule with just the exactly value (you can custom as necessary based on other CSS rules on your active policy):

<css-rules>
    ...
    <property name="mso-line-height-rule" description="Vendor property.">
        <category-list>
            <category value="visual" />
        </category-list>
        <literal-list>
            <literal value="exactly" />
        </literal-list>
    </property>
</css-rules>

is there an option to "continue the validation if a property is invalide" ?

No, I wish that exists. When the CSS parser (Batik-CSS library) encounters and reports an error, it leaves its internal "cursor" in a state that results in ending the CSS parse process. Although AntiSamy can override methods for certain parsing events, it cannot influence on how the parsing is done :(

is there a way to validate media queries ? ( @media only screen and (max-width: 480px) { )

The parser tries to validate them, it is supported. But I just don't know if the parser is bad/outdated or people use non-standard media definitions. Anyway, the support for this from Batik-CSS seems to be very basic, it was pointed out in #108. Sorry we can't provide a better solution for this :/

@optyler
Copy link
Author

optyler commented Dec 20, 2021

Great answer, thanks for the full explanation :)

We will look on how we could improve / address our issues with those new data.

Thank again

@spassarop
Copy link
Collaborator

Related to #293

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

No branches or pull requests

3 participants