thomasmueller commented on code in PR #550: URL: https://github.com/apache/jackrabbit-oak/pull/550#discussion_r857640876
########## oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LuceneJournalPropertyBuilder.java: ########## @@ -93,6 +93,9 @@ public void addSerializedProperty(@Nullable String json) { } if (sizeWithinLimits()) { indexedNodes.put(path, reader.readString()); + } else { + // return if max limit reached for builder to avoid overflow exception + return; Review Comment: I can see why the old code is broken: if the size is exceeded, we don't call readString(), but continue and try to read a comma... But I also don't like the new code: we wouldn't detect any errors in the Json format afterwards... Which I would expect we do. Instead of return, what about: ``` String x = reader.readString() if (sizeWithinLimits()) { indexedNodes.put(path, x); } ``` Sure, one could argue it is more efficient to return, but I think the side effect of _not_ parsing the rest in this case is worse than the performance hit (which is unlikely to be measurable). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org