[WG: Sakai QA] [Building Sakai] 2.6.2: SAK-17171 (Botimer vs Botimer)

[Stephen Marquard]

We should ideally have "one right way" to handle content validation and escaping in Sakai, from input through to storage and output.

In general I would say this should be:

Plain text: accept any input, store as is, escape to html markup on output using escapeHtml (e.g. > to >)
Formatted text: validate using processFormatted text on input, store as validated ("safe") html, output as-is. 

T&Q is the most complex case, because it has input fields that can be entered either as plain text or formatted. This is really a special case of storing formatted, text, viz.

Input as plain text, escape to html markup for storage, output as-is
Input as formatted text, validate using processFormattedText, store as validated ("safe") html, output as-is

Unfortunately different tools handle it in different ways, with some escaping on input and others escaping on output. This is safe so long as the model is consistently applied but as noted, potentially unsafe is the assumption about whether stored content is validated or not and/or escaped or not is changed.

If fixes do change the assumptions about stored content, then they should come with a conversion step (for example if it was previously assumed that content would be escaped on output and that's no longer the case because content is assumed stored as "safe" html, then the conversion should pass all stored content through processFormattedText to validate it).

Regards
Stephen
 
>>> Anthony Whyte <arwhyte at umich.edu> 12/22/2009 9:08 PM >>> 
Concerns have been raised relative to the proposed solution for  
SAK-17171 first noted in Samigo and later in chat and msgcntr wherein  
text input including unbalanced tag-like characters (e.g., less than/ 
greater than characters (e.g., "<", ">")) result in string index out  
of range exceptions when processed/validated by  
FormattedText.processFormattedText().  It has been suggested that the  
problem originates in a failure at the tool level to distinguish  
properly between input intended as plain text (e.g., a > b) and rich  
text (e.g., HTML).

Stephen Marquard argues in KNL-66 that plain text input should be  
escaped via Validator.escapeHTML while rich text should be processed  
and validated by FormattedText.processFormattedText().  This is the  
approach adopted in the SAK-17171 patch provided by Noah Botimer.  If  
I'm reading the Jira comments correctly, Noah has since backed away  
from his patch, describing it in SAK-17171 as the "wrong approach."   
Aaron Zeckoski agrees, arguing that the approach represents "a  
fundamental change in the way data is stored. It is definitely no  
longer a simple bug fix if you change the stored data or the way the  
data is stored and is probably inappropriate for a merge into a .x  
branch. I would encourage finding a solution which does not change the  
way data is stored if that is possible."

Escaping plain text data intended for storage appears problematic to  
me (while escaping it when outputting it to the browser does not).   
Given the debate here (if I've summarized it correctly) I'm holding  
off merging the 2.6.x patch for SAK-17171 until we sort this out.

One fix we should consider implementing is providing  
FormattedText.processFormattedText() with a friendly error message if  
text with unbalanced tags are encountered.

Anth



kernel-1.0.12 JavaDoc
http://source.sakaiproject.org/release/kernel/1.0.12/sakai-kernel-util/apidocs/org/sakaiproject/util/FormattedText.html#processFormattedText(java.lang.String,%20java.lang.StringBuilder,%20boolean,%20boolean)
http://source.sakaiproject.org/release/kernel/1.0.12/sakai-kernel-util/apidocs/org/sakaiproject/util/FormattedText.html#escapeHtml(java.lang.String,%20boolean)
http://source.sakaiproject.org/release/kernel/1.0.12/sakai-kernel-util/apidocs/org/sakaiproject/util/Validator.html#escapeHtml(java.lang.String)

More info:
kernel: http://jira.sakaiproject.org/browse/KNL-66
msgcntr: http://jira.sakaiproject.org/browse/SAK-17171
samigo: http://jira.sakaiproject.org/browse/SAK-14153
_______________________________________________
sakai-dev mailing list
sakai-dev at collab.sakaiproject.org
http://collab.sakaiproject.org/mailman/listinfo/sakai-dev

TO UNSUBSCRIBE: send email to sakai-dev-unsubscribe at collab.sakaiproject.org with a subject of "unsubscribe"