Page MenuHomePhabricator

let pht expect PhutilSafeHTMLProducerInterface
ClosedPublic

Authored by avivey on Jul 7 2016, 12:05 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 11, 8:02 AM
Unknown Object (File)
Thu, Apr 4, 5:59 PM
Unknown Object (File)
Fri, Mar 29, 1:28 PM
Unknown Object (File)
Mar 16 2024, 1:24 AM
Unknown Object (File)
Jan 22 2024, 4:49 PM
Unknown Object (File)
Jan 6 2024, 1:58 PM
Unknown Object (File)
Jan 3 2024, 6:54 PM
Unknown Object (File)
Dec 30 2023, 6:15 PM
Subscribers

Details

Summary

phutil_escape_html can handle these objects, but pht needs to know to call it.

Test Plan

Render a couple of things? I'm not sure how much damage this can do.

Diff Detail

Repository
rPHU libphutil
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

avivey retitled this revision from to let pht expect PhutilSafeHTMLProducerInterface.
avivey updated this object.
avivey edited the test plan for this revision. (Show Details)
avivey added a reviewer: epriestley.

You've mentioned we might need this in D16236, and my quick test worked, but that was because I also had a renderAuthor() in my test input, which triggered the "this is html" flag.

Alternatively, I can just add another ->render() in D16236, which is there in the existing Transaction's renderHandle() (User's renderHandle() doesn't "render" HTML, but produces a PHUIView).

Or maybe just pretend everything is HTML in this function, so we escape all inputs?

epriestley edited edge metadata.

We should possibly make PhutilSafeHTML implement PhutilSafeHTMLProducerInterface (just as return $this;, I think) to simplify this and a couple of other checks, but that's just a tiny implementation detail.

Calling render() means we have to load the handle immediately, so that's not desirable.

I believe escaping only when necessary is a (possibly significant?) optimization.

The data access pattern here is probably still not great -- I think pht(..., $this->renderHandle()) will end up loading the handle before pht() returns. But that's fixable (by making pht() return an object which evaluates lazily, at least some of the time) as long as we don't call render() inline.

This revision is now accepted and ready to land.Jul 7 2016, 12:15 AM
This revision was automatically updated to reflect the committed changes.