Re: K3 next steps / brain dump

Bharat Mediratta <[email protected]> Thu, 4 Apr 2013 17:09:50 -0700
Newsgroups gmane.comp.web.gallery.devel
Message-ID <CAESa+_ndU4wDLVdh_VtESasReTtME3LCBBMJzDth+_tYhcZnEA@mail.gmail.com>
I take it back - SafeString still has a purpose.  It adds the following
transforms:

for_html()             // HTML::chars version of the string
for_js()                // json_encoded result - safe for use in JS
for_html_attr()      // single and double quotes escaped
purified_html()      // run through HTMLPurifier
unescaped()         // the raw version

We'll still want all these cases.  For now we should keep SafeString (and
all of its associated tests!) and worry about removing/simplifying later.



On Tue, Apr 2, 2013 at 10:30 AM, Bharat Mediratta <[email protected]>wrote:

>
> Ah, I forgot to mention that!  I'm not 100% sure, but yes - I *think*that this will let us eliminate SafeString which has some minor, positive
> side-effects, like we can stop casting SafeString values to (string) in our
> code when we call functions like json_encode which can take mixed
> parameters.
>
>
> On Tue, Apr 2, 2013 at 9:46 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>
>> Cool!  While on the subject, what would/wouldn't this do to
>> SafeString?  Is it effectively replaced or... ?
>>
>> Shad
>>
>>
>> On 2 April 2013 18:28, Bharat Mediratta <[email protected]> wrote:
>> >
>> > All sounds great to me!
>> >
>> >
>> > On Tue, Apr 2, 2013 at 9:23 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>> >>
>> >> Hey Bharat,
>> >>
>> >> On 2 April 2013 01:28, Bharat Mediratta <[email protected]> wrote:
>> >> >
>> >> > On Fri, Mar 29, 2013 at 10:22 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]>
>> wrote:
>> >> >>
>> >> >> Bharat's todo before gallery can run:
>> >> >> - K2->K3 replacement: ORM and ORM_MPTT
>> >> >> - K2->K3 replacement: Database
>> >> >> - K2->K3 replacement: Cache
>> >> >> - bootstrap.php and index.php (note: I did the class name
>> >> >> search'n'replace
>> >> >> thing here)
>> >> >> - Routing
>> >> >
>> >> >
>> >> > Been busy for the past week, so progress has been slow here. I keep
>> >> > finding
>> >> > all these ratholes that I'm trying to sidestep to just Get Things
>> >> > Working.
>> >> > Oy.  I'll keep plugging away at it.
>> >>
>> >> No kidding on the ratholes!  Today I dropped in a hacked up version of
>> >> K2's Input library and fired it up for the first time.  I managed to
>> >> catch and fix a few more things, and it seems to get a bit further
>> >> along, but still not there (HTTP_Exception_302 doesn't fire a redirect
>> >> for some reason).
>> >>
>> >>
>> >> >>
>> >> >> The problem is that decamelize looks for [a-z][A-Z], which doesn't
>> work
>> >> >> with numbers.  One approach is to just do an extension like I did
>> >> >> before
>> >> >> (e.g. a new $num_sensitivity argument that changes it to
>> >> >> [a-z0-9][A-Z]).
>> >> >> That said, this is starting to make we wonder if extending the
>> built-in
>> >> >> functions is really a good approach and if we wouldn't be better
>> served
>> >> >> with
>> >> >> two purpose-built functions instead.  Thoughts?
>> >> >
>> >> >
>> >> > Sadly, I think that custom built functions are probably the right
>> way to
>> >> > go
>> >> > here.  Extending their framework and changing these APIs may break
>> >> > assumptions in the Kohana framework code.
>> >>
>> >> Agreed - new functions up and pulled.
>> >>
>> >>
>> >> >>
>> >> >> Pagination
>> >> >>
>> >> >> This library was originally in Kohana 2.x, but left after 2.3, so we
>> >> >> put
>> >> >> it in kohana23_compat for 2.4.  Similarly, it was originally in
>> Kohana
>> >> >> 3.x
>> >> >> but left after 3.1.  Some folks forked the module and made it
>> >> >> compatible
>> >> >> with 3.2 (https://github.com/kloopko/kohana-pagination), and others
>> >> >> forked
>> >> >> that to make it work with 3.3
>> >> >> (https://github.com/webking/kohana-pagination).  Does it seem like
>> >> >> pulling
>> >> >> this in as another module is a reasonable idea?
>> >> >>
>> >> >> BTW, this is the only thing left in the kohana23_compat module...
>> >> >
>> >> >
>> >> > I haven't looked it over carefully - but let's try it and see how it
>> >> > goes.
>> >> > Pagination was a bit of a beast in K2 because it expected a certain
>> >> > structure that we didn't always have.  If this pagination module
>> sucks,
>> >> > it
>> >> > might make sense to just write our own.  But let's start with their
>> code
>> >> > for
>> >> > now.
>> >>
>> >> Sounds like a plan.
>> >>
>> >>
>> >> >>
>> >> >> Input
>> >> >>
>> >> >> This library is gone with K3.  Instead, they recommend working with
>> >> >> $_GET,
>> >> >> $_POST, and $_SERVER directly.  Handling unset and default values
>> can
>> >> >> be
>> >> >> handled using Arr as:
>> >> >> Arr::get($_POST, "foo", "default foo");
>> >> >>
>> >> >> This part is pretty straightforward.  The only catch is that they
>> need
>> >> >> cleaning first.
>> >> >>
>> >> >> It was recommended to use Security::xss_clean() with K3.0, but
>> that's
>> >> >> since disappeared.  I'm looking for the best alternative, which
>> >> >> shouldn't be
>> >> >> too hard to find, but think the bigger question is this: *where* do
>> we
>> >> >> do
>> >> >> this so nobody downstream has to think about it?
>> >> >> - bootstrap?
>> >> >> - an extension of Route?
>> >> >> - an extension of Controller?
>> >> >> - somewhere else?
>> >> >
>> >> >
>> >> > The XSS cleaner in K2 was questionable at best.  We augmented it with
>> >> > our
>> >> > own tweaks:
>> >> >
>> >> >
>> https://github.com/gallery/gallery3/blob/master/system/libraries/Input.php#L306
>> >> >
>> >> > Even that is pretty crappy and I don't trust it.  We really need to
>> be
>> >> > very
>> >> > suspicious of any content in the database and treat it very carefully
>> >> > when
>> >> > we use it.  For K2 we overloaded html::clean to make it safe:
>> >> >
>> >> >
>> https://github.com/gallery/gallery3/blob/master/modules/gallery/helpers/MY_html.php#L32
>> >> >
>> >> > Then we used that (and all the other cleaning methods) everywhere.
>>  My
>> >> > assumption has always been that with html::clean AND the XSS cleaning
>> >> > code
>> >> > we don't have to catch every single vector of attack, we've got
>> enough
>> >> > defense-in-depth to make hacking hard.
>> >> >
>> >> > The downside, though is that we are always possibly cleaning away
>> >> > something
>> >> > that we really care about.  And cleaning is expensive.  :-/
>> >> >
>> >> > So options:
>> >> > 1) Don't bother cleaning at all
>> >> > 2) Create our own simple API for getting GET/POST/SERVER variables
>> that
>> >> > does
>> >> > cleaning and handles defaults
>> >> > 3) Clean everything up front
>> >> >
>> >> > I'm in favor of #2 with an API like this:
>> >> >
>> >> > Input::GET("foo", "default foo");
>> >> > Input::GET("foo", "default foo", Input::RAW);  // don't sanitize
>> >> > Input::POST("foo", "default foo");
>> >> > Input::SERVER("foo", "default foo");
>> >> >
>> >>
>> >> Me too.  I've been taking a look at how we'd integrate the standalone
>> >> HTMLPurifier, and it looks like we can be a bit faster if we use a
>> >> singleton instance() function.  With this, we could *exactly* copy the
>> >> old API (or at least the parts that Gallery used).  The instance would
>> >> initialize the class, fire up and configure HTMLPurifier once, and set
>> >> up an empty cache of purified entries.  Then, we purify them on demand
>> >> and stick them in the cache so we shouldn't need to do it twice.  I
>> >> haven't yet figured out if it makes sense to do them one superglobal
>> >> array at a time or one entry at a time, but certainly it doesn't make
>> >> sense to do all at once (for example, one might load Input to look for
>> >> $_POST but never care about $_SERVER).
>> >>
>> >> I also like the Input::RAW idea.  It should come with a big
>> >> disclaimer, and really shouldn't be needed too often since we can
>> >> accept HTML markup, but it's still nice to have.
>> >>
>> >> > This has some advantages:
>> >> > 1) It's close to what we have now so conversion is easier
>> >> > 2) We can lazy-clean the input for efficiency
>> >> > 3) We can use Input::RAW as a findable symbol for all places where we
>> >> > work
>> >> > with raw data (set the RAW constant to some wacky value so that API
>> >> > users
>> >> > have to use the constant)
>> >> > 4) profit?
>> >>
>> >> Yes, yes, yes, and hell yes.  :-)
>> >>
>> >>
>> >> > Regarding HTMLPurifier - it added so much weigh to the installed code
>> >> > size
>> >> > that I was loathe to make it a default module.  Unless it's gotten a
>> lot
>> >> > smaller, I suspect I'll still feel the same way...
>> >>
>> >> The standalone version you found seems to be not *too* bad.  It's <1MB
>> >> (<300k compressed) and seems like it was designed to play nice with
>> >> caches.
>> >>
>> >> My feeling is this: we should build the Input class and make it use
>> >> HTMLPurifier.  Then, once we're back in fully-running state, we can
>> >> run some benchmarks to see how heavy it really is.
>> >>
>> >> Thoughts?
>> >>
>> >> Take care,
>> >> Shad
>> >>
>> >>
>> >> >
>> >> > thoughts?
>> >> > -Bharat
>> >> >
>> >>
>> >
>>
>>
>

------------------------------------------------------------------------------
Minimize network downtime and maximize team effectiveness.
Reduce network management and security costs.Learn how to hire 
the most talented Cisco Certified professionals. Visit the 
Employer Resources Portal
http://www.cisco.com/web/learning/employer_resources/index.html

__[ g a l l e r y - d e v e l ]_________________________

[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]