KaTeX
KaTeX copied to clipboard
feat: add \htmlAttr command
This pull request is related to issue #2265. In #2265, I suggested adding \htmlAttr
, so that we can add any HTML attributes into a single enclosing <span>
tag. This pull request includes the \htmlAttr
that I suggested in #2265. My implementation is a very minor modification of the existing \htmlData
command.
Hey @junghoocho,
Thanks for the PR! Mind signing our Contributor License Agreement?
When you've done so, go ahead and comment [clabot:check]
and I'll check again.
Yours truly, khanbot
[clabot:check]
CLA signature looks good :+1:
If this command is considered too dangerous because it allows an arbitrary attribute to be inserted, we can add an explicit check of the added attribute names, so that only id, class, style, and data- attributes are added. What I hope to get is to have a mechanism so that all needed attributes are included in a single enclosing span tag.
I looked at the MDN doc on the span element to see on what attributes an attacker may potentially insert JavaScript code. From my review, it seems clear that event-handler attributes with names like onX
are the only places with potential JavaScript code. In my third commit 4bc5626d9508b92d4ba66c63f6dd6daa496d9d0f, I added an explicit check to disallow setting any attributes starting with on via \htmlAttr
. Of course, since \htmlAttr
has its own TrustContext, people can configure exactly what attributes are allowed through the command, but I suspect we don't want to allow inserting JavaScript through KaTeX command under any scenario. I hope this will make the addition of \htmlAttr safe enough to be merged to the mainline.
I'm against adding \htmlAttr
as (1) validation of attributes is impossible and (2) it may open to vulnerabilities and blacklisting is not enough.
To make \htmlAttr
logically equivalent to the union of existing \htmlId``\htmlClass
\htmlStyle
, and \htmlData
commands, I can add the following validation logic to \htmlAttr
:
(1) limit allowed attribute names to id
, class
, style
,data-*
(2) for id
, class
, style
, data-*
, let the attribute values pass the existing validation logics for htmlid
, htmlclass
and htmlstyle
, htmldata
values, respectively.
If you think this will be an acceptable implementation, I will be happy to add the above logic to the pull request for review.
@junghoocho That seems to be OK. If you don't mind, could you update the PR? I'll leave #2265 open for discussions on allowing other attributes.
@ylemkimon Thank you very much. I just updated the pull request to allow only id, style, class, and data- attributes in \htmlAttr
Regarding attribute-value validation, I find that the existing code does not perform any validation on its own and relies on the user-provided trust
function. Given this, the updated pull request simply calls the user-provided trust function for validation as well.
Let me know if it looks OK or if I missed anything. Thank you.