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

[Stephen Marquard]

Also I think processFormattedText should never be used for output escaping / validation, because you can't do anything about a failure like mismatched tags, whereas in the input phase, you can respond with an input / validation error.

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"