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

Thomas Mueller commented on OAK-3395:
-------------------------------------

I would simplify EscapeUtils a bit:

{noformat}
    private static boolean unescapingRequired(String line) {
        return line.indexOf('\\') >= 0;
    }
{noformat}

I would also allocate one additional character here:

{noformat}
    escape(String line) {
        int len = line.length();
        StringBuilder sb = new StringBuilder(len + 1);
{noformat}

and one fewer here:

{noformat}
    private static String unescape(String line) {
        int len = line.length();
        StringBuilder sb = new StringBuilder(len - 1);
{noformat}

For unformat, I would throw an IllegalArgumentException on invalid syntax, to 
detect bugs (missing escape calls):

{noformat}
    private static String unescape(String line) {
        int len = line.length();
        StringBuilder sb = new StringBuilder(len - 1);
        for (int i = 0; i < len; i++) {
            char c = line.charAt(i);
            if (c == '\\') {
                char nextChar = line.charAt(i + 1);
                switch (nextChar) {
                    case 'n':
                        sb.append('\n');
                        i++;
                        break;
                    case 'r':
                        sb.append('\r');
                        i++;
                        break;
                    case '\\':
                        sb.append('\\');
                        i++;
                        break;
                    default:
                        throw new IllegalArgumentException(...);
{noformat}

(by the way, even now an exception is throws if the last character if a 
backslash).

There should be unit tests for the special cases as well (code coverage should 
be 100%).

> RevisionGC fails for JCR paths having line feed characters
> ----------------------------------------------------------
>
>                 Key: OAK-3395
>                 URL: https://issues.apache.org/jira/browse/OAK-3395
>             Project: Jackrabbit Oak
>          Issue Type: Bug
>          Components: mongomk, rdbmk
>            Reporter: Chetan Mehrotra
>            Assignee: Chetan Mehrotra
>            Priority: Minor
>             Fix For: 1.3.7, 1.2.6, 1.0.21
>
>         Attachments: OAK-3395-1.patch
>
>
> RevisionGC fails with error while processing any id (derived from JCR path) 
> having line feed or carriage return char
> This happens because it relies on Oak Commons StringSort and ExternalSort 
> which works with line delimited string and having an id with line break would 
> break this sorting logic. Error reported is like
> {noformat}
> java.lang.AssertionError: Invalid id /1442211320
>       at 
> org.apache.jackrabbit.oak.plugins.document.util.Utils.getDepthFromId(Utils.java:337)
>       at 
> org.apache.jackrabbit.oak.plugins.document.NodeDocumentIdComparator.compare(NodeDocumentIdComparator.java:38)
>       at 
> org.apache.jackrabbit.oak.plugins.document.NodeDocumentIdComparator.compare(NodeDocumentIdComparator.java:30)
>       at java.util.TimSort.countRunAndMakeAscending(TimSort.java:324)
>       at java.util.TimSort.sort(TimSort.java:203)
>       at java.util.TimSort.sort(TimSort.java:173)
>       at java.util.Arrays.sort(Arrays.java:659)
>       at java.util.Collections.sort(Collections.java:217)
>       at 
> org.apache.jackrabbit.oak.commons.sort.ExternalSort.sortAndSave(ExternalSort.java:279)
>       at 
> org.apache.jackrabbit.oak.commons.sort.ExternalSort.sortInBatch(ExternalSort.java:218)
>       at 
> org.apache.jackrabbit.oak.commons.sort.ExternalSort.sortInBatch(ExternalSort.java:257)
>       at 
> org.apache.jackrabbit.oak.commons.sort.StringSort$PersistentState.sort(StringSort.java:191)
>       at 
> org.apache.jackrabbit.oak.commons.sort.StringSort.sort(StringSort.java:88)
>       at 
> org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector$DeletedDocsGC.ensureSorted(VersionGarbageCollector.java:383)
>       at 
> org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector$DeletedDocsGC.getDocIdsToDelete(VersionGarbageCollector.java:274)
>       at 
> org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector$DeletedDocsGC.removeDeletedDocuments(VersionGarbageCollector.java:296)
>       at 
> org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector$DeletedDocsGC.removeDocuments(VersionGarbageCollector.java:241)
>       at 
> org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.collectDeletedDocuments(VersionGarbageCollector.java:154)
>       at 
> org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.gc(VersionGarbageCollector.java:105)
>       at 
> org.apache.jackrabbit.oak.plugins.document.VersionGCDeletionTest.gcWithPathsHavingNewLine(VersionGCDeletionTest.java:203)
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to