[ 
https://issues.apache.org/jira/browse/SLING-2425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13256724#comment-13256724
 ] 

Alexander Klimetschek commented on SLING-2425:
----------------------------------------------

I had a quick look at the code. Generally it looks good wrt to the backwards 
compatibility and unit tests present. Only the case if the name is a path looks 
fragile with the custom parsing. For sure it does not catch relative path steps 
such as "." or "..", they will get escaped incorrectly.

Looking at that I think that not doing any encodings in the value map layer is 
much simpler - this encoding logic can get endlessly complex and I guess there 
will always be a case where it fails if the application cannot decide what to 
do (e.g. escape "/" or not).
                
> Incorrect and inconsistent escaping of property names used in JcrPropertyMap
> ----------------------------------------------------------------------------
>
>                 Key: SLING-2425
>                 URL: https://issues.apache.org/jira/browse/SLING-2425
>             Project: Sling
>          Issue Type: Bug
>          Components: JCR
>    Affects Versions: JCR Resource 2.0.10
>            Reporter: Alexander Klimetschek
>            Assignee: Carsten Ziegeler
>             Fix For: JCR Resource 2.1.0
>
>
> The JcrPropertyMap uses the (wrong) ISO9075 encoding for property names, and 
> this also behaves differently between the read() and readFully() variants.
> 1) ISO9075 is needed for XML names, e.g. for mapping JCR names into Xpath 
> queries. But the set of valid JCR names is much larger (for example "-" is 
> valid, while it is not allowed in ISO9075 and becomes "_x002d_"). 
> org.apache.jackrabbit.util.Text#escapeIllegalJcrChars() must be used instead 
> to escape any string for use as JCR names. [0]
> 2) Inconsistency:
> a) read() will take the key and use ISO9075#encodePath(), before looking up 
> the jcr property using the encoded variant
> b) readFully() will go through all jcr properties and cache them with the key 
> using ISO9075#decode()
> Hence for all valid JCR names, which are not valid under ISO9075 (like 
> "1_prop", "-foo"), these can be looked up using the cached variant b) (as 
> decode() won't touch them), while they cannot be looked up using read() at 
> all due to the forced "arbitrary" escaping.
> I think there should be no auto-magically escaping at all (also not in the 
> accompanying JcrModifiablePropertyMap). Incorrect naming errors should simply 
> be passed through, it is the job of the application to handle that. The 
> framework should not run an arbitrary & undocumented escaping, if it cannot 
> enforce that anyway, since there are other ways to create properties with a 
> different valid char set (using the JCR API).
> [0] http://wiki.apache.org/jackrabbit/EncodingAndEscaping

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to