elm-community / html-extra

Additional functions for working with Html.
http://package.elm-lang.org/packages/elm-community/html-extra/latest
MIT License
31 stars 14 forks source link

Refactor `innerHtml` to take a Trusted/Unsafe String Type #5

Closed rgrempel closed 5 years ago

rgrempel commented 8 years ago

I thought I'd break out this discussion from the tail of the PR for innerHtml : https://github.com/elm-community/elm-html-extra/pull/4

The question is whether to use the type system in some way to reflect the fact that strings provided to innerHtml should be trusted -- that is, either from trusted sources, or "cleaned up" in some way to avoid XSS attacks and the like.

@botandrose pointed out that Rails uses a SafeBuffer concept to reflect this: http://yehudakatz.com/2010/02/01/safebuffers-and-rails-3-0/

I think there are several levels of engagement possible here:

  1. Documentation-only: What we have now ... that is, the documentation for innerHtml mentions the issue.
  2. Type-level documentation: We could do something like this:

    type alias TrustedString = String
    
    innerHtml : TrustedString -> Attribute

    Since it would be an alias, you could still supply a plain-old-string, but the documentation would be reinforced in the type.

  3. Wrapper type: We could have a "wrapper" type called Trusted (or something like that), along these lines:

    type Trusted a =
      Trusted a
    
    trust : a -> Trusted a
    trust = Trusted
    
    extract : Trusted a -> a
    extract trusted =
      case trusted of
          Trusted a -> a
    
    innerHtml : Trusted String -> Attribute

    In this case, you'd actually have to explicitly trust a String to use it with innerHtml.

  4. Extended wrapper type: In fact, the wrapper type probably isn't super-attractive without a few more helper-type methods. I haven't really thought this through, but presumably you'd want at least something like this:

    map : (a -> a) -> Trusted a -> Trusted a

    And there would be more, no doubt -- the usual suspects, you might say.

Anyway, I thought I'd make this a separate issue, for discussion.

andersk commented 8 years ago

A function like map : (a -> a) -> Trusted a -> Trusted a doesn’t really help you maintain any safety. Consider map (\s -> s ++ "<script>alert(1)</script>") (trust ""). The types suggest that only the empty string is being “trusted”, but actually all possible strings generated by the function need to be “trusted”.

I put “trusted” in quotes because all “trusted” really means is “should be parsed as HTML”. This isn’t an information flow control problem to be solved by assigning trust levels to the sources of your data—it’s just a formatting problem where you always need to perform an escaping transformation in order to render text to HTML. And ideally, this entire concept would be redundant in a language where we already have such a rich vocabulary for manipulating DOM nodes. The way you should mark pre-rendered HTML as “trusted” is by immediately converting it to a DOM node.

Perhaps the issue with this argument is that the only existing way to convert String to Html msg involves wrapping it in a containing node such as a <div>, with Html.div [Html.Attributes.Extra.innerHtml s] []. That will be fine for some use cases. For the rest, I think what we really want is some way of creating document fragments? (… Now that I’ve gone and spent an hour learning how virtual-dom works, the details seem more complicated than I expected: Matt-Esch/virtual-dom#251.)

prikhi commented 6 years ago

I'm thinking extended wrapper type but with no map function. Something like

type UnsafeString = UnsafeString String
unsafe : String -> UnsafeString
innerHtml : UnsafeString -> Attribute msg
eeue56 commented 6 years ago

This seems out of scope for this repo, fwiw.

ianmackenzie commented 5 years ago

In Elm 0.19 this is no longer possible anyways since innerHtml is no longer supported, right?