[Kss-devel] Cleanup of kss.core - sanitization of string and html params

[Godefroid Chapelle]

Balazs Ree wrote:
> On Thu, 13 Mar 2008 21:58:07 +0100, Jeroen Vloothuis wrote:
> 
>> The kss.core branch which builds on kss.zope has been cleaned of all the
>> unneeded code. What is left is either tests (or related documents) or
>> bbb imports or code.
>>
>> While cleaning out kss.core I came across a few module which seem to
>> have some use but that I am uncertain of how to handle these.
>>
>> The first one is BeautifulSoup.py, this is imported from a plone.app.kss
>> directly. Next is the unicode_quirks.py file. It is used from
>> plone.app.kss as well (plonekssview.py).
> 
> I'll try to explain first, how these currently work in kss.core, and what 
> is their functionality.
> 
> BeautifulSoup.py is an html parser that is used for sanitizing the html 
> output, and its usage is enforced each time html is sent to the client. 
> This is done each time an HTML parameter is added, by the addHtmlParam 
> method, ie any time when the semantics of a parameter is "HTML".
> 
> The force_unicode method works similarly for adding parameters with a 
> "string" semantics, ie. when addString is used. This makes sure that 
> either plain ascii, or proper unicode is added (with an additional 
> parameter that makes it possible to _explicitely_ enforce conversion from 
> a given codepage other then utf8.) In case of faulty input an exception 
> is raised on the server side.
> 
>> My suggestion is to just to leave them in kss.core so that we can
>> deprecate them at some later time.
> 
> Before getting to the bbb policy at all, there is a more important 
> question that raises.
> 
> The AddHtmlParam and AddStringParam, besides implementation reason that 
> may occur differently in the case of kss.base, have a very important 
> function. We do not only handle these parameter types disctintly from 
> each other because we marshal them disctinctly on the kss command payload 
> (which we may change in the next implementation), but precisely because 
> we want to check and sanitize the parameter content on the server side. 
> This altogether improves the quality of applications based on top of kss, 
> and enables earlier caching of certain application errors.
> 
> I would like to refer to some of our earlier discussions when I tried to 
> emphasize the importance of keeping the different addXXXParam methods 
> based on the parameter's semantics, versus having a single addBlobParam 
> method. (As in the IKSSCommand interface.) One of my reasons for doing so 
> is that I am strongly on the point of view, that these sanitizations are 
> very important. I keep standing on this point still at the moment. Apart 
> from this importance, the other reason is that I believe that the switch 
> to kss.zope should not cause any observable difference to the 
> applications that are running on top of kss. It is important to nail down 
> that he goal of kss.base - although it was a complete rewrite - is not 
> redesigning the kss server side behaviour but to enable exactly the same 
> behaviour that we currently have, on the level of plain python 
> applications. We may want to do refactorizazion in the future but it must 
> occur in a consecutive step, separated from the current porting effort.
> 
> Now, if implemented properly, this functionality belongs to the level of 
> kss.base. I am not sure if this checking is implemented in kss.base and 
> on what extent, but it seems not or not identically implemented to the 
> way they currently are in kss.core. However I may be wrong at this point.
> 
> So the question that is raised: I offered some reasons above why I find 
> this functionality important and to be kept. Do we have any reason to 
> drop this functionality in the next release (based on kss.base)?

Definitely no reason. I agree that html sanitizing and unicode nicety 
need to be kept.

And that they should go to kss.base.


> And what problems may possibly arise for existing applications and Plone, 
> in particular, if we do so?
> 
> I believe we must discuss and answer these questions first, and decide 
> the implementation and deprecation policy based on the decision we make.
>  
> 
>> The buildout for the kss.zope related work has been updated to match the
>> trunk of Plone. All tests in de demo's pass and a Grok demo works.
> 
> Obviously, tests were (again) missing in this case, otherwise they would 
> not pass.
> 
> After deciding on the above mentioned question, what would follow?
> 
> If we decide to keep this checks, we must add additional tests to 
> kss.core that check for the existence of the string and html checking and 
> sanitization. Following this the tests must be moved or implemented in 
> kss.base as well (together with the actual implementation of the 
> sanitization, if missing.)
> 
> In case we decide to drop these checks, we must carefully check in real 
> life scenarios, what errors or changed behaviour we get in case broken 
> html or sting content is inserted to a kss command. If this is the case 
> we may still consider if we redistribute BeautifulSoup in kss.zope 
> because it is a very handy html parser that we used several times in 
> applications that produce html content for kss, but in this case it will 
> be just provided as a utility and not be used by kss.base/kss.core itself.

FYI, BeautifulSoup is now available as an egg.

> 
> Best wishes
> 


-- 
Godefroid Chapelle (aka __gotcha) http://bubblenet.be