Michael Blow has posted comments on this change.

Change subject: ASTERIXDB-1470 Fix escapes in String values in ClassAd Parser
......................................................................


Patch Set 3:

(3 comments)

https://asterix-gerrit.ics.uci.edu/#/c/915/3/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/classad/Util.java
File 
asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/classad/Util.java:

Line 37:     private static final Pattern OCTAL = 
Pattern.compile("\\\\([0-3][0-7]{0,2})");
Thinking on this more, I wonder if this should remain as \\d{0,3}, such that 
illegal octets are detected.  Otherwise, illegal ones will just pass with the 
backslash stripped, which seems bad?  Dunno.


Line 108:             text.erase(text.getLength() - 1);
I don't think any chars are going to be \0 here, as the replacement hasn't 
taken place yet.  I think you need to do this check in your while loop, and 
strip the character instead of return false if we're at end of string (m.end() 
== text.toString().length() maybe?)


Line 118:                 m.appendReplacement(out, String.valueOf((char) 
octal));
What if octet is > 255?  We need to throw or return false, right?


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/915
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id6e4e25263c5a6f0efe26773da7c3b8fcf7e2427
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <michael.b...@couchbase.com>
Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to