Before we lose context, I want add more of my thoughts here.
I think the priority of things should be:
1. Functionality
2. Security
3. Performance
Alex probably thinks it should be:
1. Functionality
2. Performance
3. Security
As a compromise, in this context I am going to suggest that we go with:
1. Security,
2. Performance
3. Functionality
That is, security is still higher than functionality (which is what I
want). And the priority of performance has not changed (I think what Alex
wants)
To explain it in a bit more detail: My goal is to not allow a developer to
inadvertently add a bad piece of HTML to the DOM. That is how security
exploits occur. We cannot simply say that it is upon the developer to
ensure that a HTML snippet is sanitized before they use HTMLText to render
it on the screen. We've seen this happen to Flash Player in the past where
ease of use was prioritized over security. This behavior changed
eventually, where security was considered when building each feature. I
think we should learn from that to avoid being labeled as 'insecure
software' .
The options that Alex suggested (doing it in the HTTPService layer, etc.)
does not cut it. Because:
a. It is not appropriate for HTTPService to divine the nature of a piece
of data. HTML can be used to display as DOM or shown to user to edit. We
could have two UI widgets one, that shows the HTML in an editable text
area, and one that previews the HTML. For example, editing an already
comment in JIRA or a blog post and previewing it before saving. If we
blindly let the HTTPService sanitize the data, we are losing data.
b. A scary piece of HTML is not a security threat when it lives as a
string. There is no need to do anything about it. It becomes a security
threat only when it is added to the DOM. So, sanitizing it just before it
is added to the DOM is the most appropriate thing to do. I guess this is
perfectly adhering to the 'Pay as you go' philosophy. Don't sanitize the
HTML just in case, sanitize it just in time.
So, as a compromise, here is what I propose:
By default, HTMLText component expects an IHTMLSanitizer bead. If such a
bead is not available, the component will throw a runtime exception and it
will not render the HTML.
IHTMLSanitizer probably looks like this:
function sanitizeHTML(v:String) :String;
We will have two beads built in:
1. HTMLSanitizer implements IHTMLSanitizer and uses a default algorithm to
sanitize the given HTML.
2. InsecureNoHTMLSanitizer implements IHTMLSanitizer which simply returns
the same string back.
We can build two 'express' components like this:
express:HTMLText looks like this:
<basic:HTMLText>
<basic:beads>
<basic:HTMLSanitizer />
</basic:beads>
<basic:HTMLText>
express:InsecureHTMLText looks like this:
<basic:HTMLText>
<basic:beads>
<basic:InsecureNoHTMLSanitizer />
</basic:beads>
<basic:HTMLText>
This achieves the following goals:
1. If someone wants to use basic:HTMLText and they don't provide a
IHTMLSanitizer, they wont be able to display any HTML and will encounter
runtime errors.
2. They can use express:HTMLText which will run all HTML through a
sanitizer. They will either be happy or will complain about performance.
3. They can switch to express:InsecureHTMLText and get better
performance. But since the name of the component clearly says that using
it is insecure, we have at least given them an opportunity to understand
what is going on.
4. Some folks might want their own HTML sanitization logic. They can
create their own SpecialHTMLSanitizer which implements IHTMLSanitize and
add it to basic:HTMLText
Thoughts?
Thanks,
Om
P.S. Does the name 'TrustedHTMLText' work better than 'InsecureHTMLText'?
On Tue, Jan 3, 2017 at 8:20 AM, flex capacitor <flexcapaci...@gmail.com>
wrote:
> I have a few thoughts on that. I need a component like this in the past for
> the times I had separate HTML content already created but needed to be
> integrated into a Flex JS app. I would suggest a contentBefore,
> contentAfter and contentOverride properties maybe as a bead or a full
> separate component.
>
> Those would be child tag properties that would pass the raw content through
> enclosed in CDATA tags or not and the container or bead would have a
> sanitize properties that could be set to true or false.
>
> The purpose would be to allow HTML embed content. Say you want to drop a
> Google maps widget or rich text editor into your Flex JS app. You don't
> want the compiler to parse that code. You would just want app to write the
> raw HTML to the document in the container it is declared in. You would be
> responsible for the security as the author and how to talk to it.
>
> The before and after properties would allow things like HTML comments or
> other uses. In the past you had php code that wraps HTML markup but now it
> could be used for some of the other frameworks out there to use markup to
> perform other tasks.
>
> How that will work with the Flex JS model new App(); I'm not sure.
>
> On Jan 1, 2017 3:16 AM, "OmPrakash Muppirala" <bigosma...@gmail.com>
> wrote:
>
> > On Sun, Jan 1, 2017 at 12:15 AM, Alex Harui <aha...@adobe.com> wrote:
> >
> > >
> > >
> > > On 12/31/16, 11:23 PM, "omup...@gmail.com on behalf of OmPrakash
> > > Muppirala" <omup...@gmail.com on behalf of bigosma...@gmail.com>
> wrote:
> > > >
> > > >Okay, I see your point now. To be clear, I did not set out to write
> > HTML
> > > >code. The bio in team.json is a html snippet. So, I am just trying
> to
> > > >display it as a piece of formatted data. Even in other projects, I do
> > > >need
> > > >this kind of thing quite often.
> > >
> > > IMO, this is use case #1: non-interactive HTML content. We could just
> > > have an HTML component with an "html" property. But it would want to
> > wrap
> > > the content in a <div> or <span>
> > >
> >
> > This is all I wanted. I implemented this and it seems to work fine.
> >
> >
> > >
> > > Case #2: In MXML, someone wants to mix FlexJS elements with random
> HTML.
> > >
> > > Case #3: In MXML, someone wants to put in random HTML where one child
> > HTML
> > > tag has an id and wants to write code that addresses that id.
> > >
> > > The cheap option is to only support #1 by creating an HTML component
> with
> > > an "html" property. Everything else needs more thinking, IMO.
> > >
> >
> > 2 and 3 sound like terrible ways to code :-)
> >
> >
> > >
> > > >
> > > >That brings up a related concern. We need to sanitize such html
> content
> > > >during runtime [1]
> > >
> > > IMO, those are just utility functions. Not everybody needs
> sanitization,
> > > IMO, so PAYG.
> > >
> >
> > Hmm, to play the devil's advocate, security should not be pay-as-you-go.
> > This should be opt-in by default. Someone will have to go the extra mile
> > to turn it off.
> >
> > This is the sort of thing that will go out in the wild and folks will get
> > affected by it soon enough. We will then need to push out an emergency
> > release to fix an XSS attack made possible by FlexJS.
> >
> > Either that or we call the default implementation 'InsecureHTMLText' or
> > something like that.
> >
> >
> > >
> > > >>
> > > >>Not really sure why this is required. I am looking for usage
> patterns
> > > >>like
> > > >>this:
> > > >>
> > > >><Panel>
> > > >> <HTMLText>{myFancyHTMLText}</HTMLText>
> > > >> <Image></Image>
> > > >></Panel>
> > > >>
> > > >><TabNavigator>
> > > >> <Tab>
> > > >> <Button />
> > > >> <HTMLText />
> > > >> </Tab>
> > > >> <Tab><HTMLText></Tab>
> > > >></TabNavigator>
> > > >>
> > > >>I guess the HTMLText component has to be a UIBase for this to work,
> > > >>right?
> > > >>
> > > >>Maybe I am not understanding the point you are trying to make.
> > > >
> > > >The above would likely create an HTML DOM like:
> > > >
> > > ><Div className="Panel">
> > > > <Span>the HTML from myFancyHTMLText</Span>
> > > > <Img />
> > > ></Div>
> > > >
> > > >
> > > >Carlos asked earlier about getting rid of the Span since we all know
> it
> > is
> > > >legit to have the HTML DOM look like:
> > > >
> > > ><Div className="Panel">
> > > > The HTML from MyFancyHTMLTExt
> > > > <img />
> > > ></div>
> > > >
> > > >
> > > >I doubt if this HTML can be generated. Are you sure there will be a
> > > >linebreak after the text?
> > >
> > > No. I just added that to show the "children".
> > >
> > > >
> > > >That looks a TextNode element. Which is lightweight than innerHTML
> But
> > > >are we sure we don't want formatting for such text? If we want to be
> > able
> > > >to format, we need to use a p or a span element.
> > >
> > > AIUI, a TextNode is just plain text. If the above was:
> > >
> > > <div className="Panel">
> > > The <b>HTML</b> from <strong>MyFancyHTMLTExt</strong>.
> > > <img />
> > > </div>
> > >
> > > There would be a TextNode for "The", a Bold node for "HTML", another
> > > TextNode for "from" etc.
> > >
> >
> > What I meant was that HTML could be achieved by using the
> > document.createTextNode() function which creates a TextNode element.
> i.e.
> > no need for a wrapping element like P or Span. Sorry for the confusion.
> >
> >
> > >
> > >
> > > This doesn't feel right to me. All of us seem tempted to want to
> define
> > a
> > > scrap of complex HTML separate from other child controls in a
> container.
> > > If there was an HTML control, it would still add a wrapping element.
> If
> > > you pull of a trick where the HTML control just injects the content,
> you
> > > run into more complexity dealing with numElements and childIndex. It
> > > would be what the browser says it would be, but not quite as obvious
> from
> > > the MXML.
> > >
> > > On the other hand we have those thin-wrapping components like Div, and
> A
> > > and you ought to be a able to put in HTML content without it being
> > wrapped
> > > by another layer.
> > >
> >
> > Yes, I am in agreement with you here. A div or span with innerHTML is
> more
> > than adequate for this usecase.
> >
> >
> > >
> > > >There is a cost in both development time and runtime performance to
> > > >generalizing this. Should we do it?
> > > >
> > > >
> > > >Hmm, I don't think this requires a change to MXML parsing.
> > >
> > > I think it does. The default property is expected to be an array of
> > > Ichild elements. When you have random HTML in the default property the
> > > compiler won't accept XML TextNodes in many places and where it does,
> it
> > > tries to concatenate TextNodes which is what we don't want.
> > >
> >
> > I see. I did not realize this point. In any case, I would advice not
> > adding a random TextNode element like this. This also brings more
> security
> > issues [1]
> >
> > Thanks,
> > Om
> >
> > [1] http://benv.ca/2012/10/02/you-are-probably-misusing-DOM-
> text-methods/
> >
> >
> >
> > >
> > > Other opinions welcome...
> > > -Alex
> > >
> > >
> >
>
