Skip to content

Add innerHtml function to enable inserting arbitrary HTML#4

Merged
mgold merged 1 commit into
elm-community:masterfrom
botandrose:innerHtml
Apr 11, 2016
Merged

Add innerHtml function to enable inserting arbitrary HTML#4
mgold merged 1 commit into
elm-community:masterfrom
botandrose:innerHtml

Conversation

@botandrose
Copy link
Copy Markdown
Contributor

Howdy!

I've copying and pasting this little gem around for a few projects now, so I think its time to extract it into its own package. I think it might be broadly useful enough to warrant inclusion in this package. What do you think?

@lukewestby
Copy link
Copy Markdown

+1 from me

cc @elm-community/packages

@eeue56
Copy link
Copy Markdown

eeue56 commented Apr 7, 2016

update comment to note that it doesn't convert them to vdom nodes

@botandrose
Copy link
Copy Markdown
Contributor Author

@eeue56 Good call. Added, squashed, and force pushed. WDYT?

@mgold
Copy link
Copy Markdown
Member

mgold commented Apr 8, 2016

Seems like a security hole, worth documenting?

@rgrempel
Copy link
Copy Markdown
Member

rgrempel commented Apr 8, 2016

Probably worth a caution about "untrusted input" ... that is, you wouldn't want to insert innerHtml that just anyone can enter.

@botandrose
Copy link
Copy Markdown
Contributor Author

Struggling with the best way to word this. Perhaps appending "Also note that passing untrusted input to this function could enable XSS attacks."?

@lukewestby
Copy link
Copy Markdown

I'm not sure it should necessarily fall on this package to document that concern. This function is just a wrapper for something you can already do and folks are already doing with elm-html.

@laszlopandy
Copy link
Copy Markdown

I would even go further and suggest that the property name should be changed to indicate the security risks, naming it dangerouslySetInnerHTML like React does:
https://facebook.github.io/react/tips/dangerously-set-inner-html.html

@mgold
Copy link
Copy Markdown
Member

mgold commented Apr 11, 2016

On the theory that some action is better than inaction, I'm going to merge this and then add a security warning to the docs.

@mgold mgold merged commit 4072ee0 into elm-community:master Apr 11, 2016
mgold added a commit that referenced this pull request Apr 11, 2016
@botandrose botandrose deleted the innerHtml branch April 11, 2016 16:06
@botandrose
Copy link
Copy Markdown
Contributor Author

Looks good to me. Thanks, Max!

@rgrempel
Copy link
Copy Markdown
Member

One other idea occurs to me -- it may be overkill (in fact, it probably is overkill).

Ruby has this concept of things being "tainted" or "trusted", in order to mark whether they come from.

We could adapt that idea to the type signature. One version would be something like this:

type alias TrustedString = String

innerHtml : TrustedString -> Attribute

This would actually be fairly non-intrusive to the caller, since you could still provide a plain-old-String ... the TrustedString in the signature would merely be a kind of documentation.

The other approach would be to actually make Trusted a real type ... something like:

type Trusted a =
    Trusted a

trust : a -> Trusted a
trust = Trusted

extract : Trusted a -> a
extract trusted =
    case trusted of
        Trusted a -> a

Then, the type signature would be something like:

innerHtml : Trusted String -> Attribute

So, this would be more intrusive, in the sense that you'd have to explicitly trust your strings, but in a way that's also it's benefit.

So, I just throw it out there as a possible idea -- as I say, it may well be overkill.

@botandrose
Copy link
Copy Markdown
Contributor Author

Yeah, I've actually been thinking about the same thing... that Rails'
SafeBuffer approach might lend itself really well to Elm, because I think
we could leverage the type system to do most of the work of enforcing XSS
safety. For reference:
http://yehudakatz.com/2010/02/01/safebuffers-and-rails-3-0/

Probably out of the scope of this PR, but it's definitely something I've
been thinking about.

On Mon, Apr 11, 2016, 9:31 AM Ryan Rempel notifications@github.com wrote:

One other idea occurs to me -- it may be overkill (in fact, it probably is
overkill).

Ruby has this concept of things being "tainted" or "trusted", in order to
mark whether they come from.

We could adapt that idea to the type signature. One version would be
something like this:

type alias TrustedString = String

innerHtml : TrustedString -> Attribute

This would actually be fairly non-intrusive to the caller, since you could
still provide a plain-old-String ... the TrustedString in the signature
would merely be a kind of documentation.

The other approach would be to actually make Trusted a real type ...
something like:

type Trusted a =
Trusted a
trust : a -> Trusted atrust = Trusted
extract : Trusted a -> aextract trusted =
case trusted of
Trusted a -> a

Then, the type signature would be something like:

innerHtml : Trusted String -> Attribute

So, this would be more intrusive, in the sense that you'd have to
explicitly trust your strings, but in a way that's also it's benefit.

So, I just throw it out there as a possible idea -- as I say, it may well
be overkill.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#4 (comment)

@rgrempel
Copy link
Copy Markdown
Member

I wrote up an issue re: using types for trusted string, for further discussion: #5

@botandrose botandrose restored the innerHtml branch November 16, 2017 00:14
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

Successfully merging this pull request may close these issues.

7 participants