[
https://issues.apache.org/jira/browse/OPENNLP-1955?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Martin Wiesner updated OPENNLP-1955:
------------------------------------
Summary: Replace literal String.split calls with a regex-free
StringUtil.split (was: Replace literal String.split calls in production code
with a regex-free StringUtil.split)
> Replace literal String.split calls with a regex-free StringUtil.split
> ---------------------------------------------------------------------
>
> Key: OPENNLP-1955
> URL: https://issues.apache.org/jira/browse/OPENNLP-1955
> Project: OpenNLP
> Issue Type: Improvement
> Reporter: Kristian Rickert
> Assignee: Kristian Rickert
> Priority: Major
> Time Spent: 50m
> Remaining Estimate: 0h
>
> OpenNLP production code must not use regular expressions in the processing
> path; user-supplied regex steps (RegexNameFinder) are the documented
> exception (OPENNLP-1925). Epic OPENNLP-1926 removed the {{java.util.regex}}
> imports, but the audit keyed on the
> import and missed literal-separator {{String.split}} calls, which never
> import the package. 33 production files still call {{{}split(","){}}},
> \{{split("")}}, {{{}split("\t"){}}}, {{{}split("-"){}}}, {{{}split("_"){}}},
> {{{}split(";"){}}}, {{{}split(":"){}}}, {{{}split("#"){}}}, or
> {{{}split("\n"){}}}. The JDK fast-paths single-character separators, so this
> is a policy and consistency fix, not a performance fix. A shared utility also
> gives the future no-regex guard (OPENNLP-1935, forbiddenapis direction) a
> sanctioned replacement, which is what makes a complete ban on
> {{String.split}} enforceable.
> h2. Change
> Add to {{opennlp.tools.util.StringUtil}} (opennlp-api):
> * {{{}split(CharSequence, char){}}}: byte-identical semantics to
> {{String.split}} with a literal single-character pattern and limit 0: leading
> and middle empty fields kept, trailing empty fields removed, empty input
> yields {{{}[""]{}}}. No regex, no Pattern,
> code-point safe.
> * {{{}split(CharSequence, char, int limit){}}}: same with the
> {{String.split}} limit contract (needed by the Emoji and FullCaseFold table
> loaders, which call {{{}split(FIELD_SEPARATOR, 5){}}}).
> Both are fuzz-verified byte-identical against {{String.split}} over random
> inputs: empty, leading/trailing/repeated separators, supplementary code
> points, limit values 0, 1, 2, 5, and negative.
> Then replace the calls, verifying each site has a {{String }}receiver:
> * opennlp-api: ExtensionLoader
> * opennlp-cli: FineGrainedReportListener (first-field extraction becomes an
> indexOf walk, no array), DoccatSVMTrainerTool, DoccatTrainerTool,
> NGramLanguageModelTool, TokenNameFinderCrossValidatorTool,
> TokenNameFinderEvaluatorTool, TokenNameFinderTrainerTool,
> SentimentEvaluatorTool
> * opennlp-formats: BioNLP2004NameSampleStream, Conll02NameSampleStream,
> Conll03NameSampleStream, ConllXPOSSampleStream, EvalitaNameSampleStream,
> GermEval2014NameSampleStream, ad/PortugueseContractionUtility,
> brat/BratAnnotationStream, brat/BratNameSampleStreamFactory,
> conllu/ConlluWordLine, masc/MascSentenceParser,
> nkjp/NKJPSegmentationDocument, ontonotes/DocumentToLineStream
> * opennlp-runtime: ChunkSampleStream, DoccatFactory,
> DictionaryLemmatizer, LemmaSampleStream, POSDictionary,
> WordClusterDictionary, EmojiAnnotations, EmojiEmoticons,
> FullCaseFoldCharSequenceNormalizer, Glove
> * opennlp-extensions: uima AbstractNameFinder
> All of these are internal parsing, not regex behavior promised by a public or
> protected API, so they are corrected and tested without a migration path.
> Whitespace splitting is out of scope
> ({{{}StringUtil.splitOnUnicodeWhitespace{}}}, OPENNLP-1860/OPENNLP-1929), as
> are the exempt regex steps (RegexNameFinder, Parse, NameSample,
> AncoraSpanishHeadRules, SpellCheckingCharSequenceNormalizer) and the public
> Pattern contract of TokenizerFactory/lang.Factory.
> h2. Acceptance
> * Zero
> {{{}String.split{}}}/{{{}matches{}}}/{{{}replaceAll{}}}/{{{}replaceFirst
> {}}}calls remain in the listed files; existing tests pass.
> * New tests pin the split contract, including the trailing-empty and
> empty-input cases, and the fuzz equivalence evidence is quoted in the commit
> message.
> * Test-first: pinning tests for the replaced parsing behavior land before or
> with the replacement.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
