[
https://issues.apache.org/jira/browse/OPENNLP-1929?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18120440#comment-18120440
]
ASF GitHub Bot commented on OPENNLP-1929:
-----------------------------------------
krickert commented on PR #1280:
URL: https://github.com/apache/opennlp/pull/1280#issuecomment-5881535258
Thanks. I squashed everything with your patch applied. The migration notes
are in the manual paragraph in machine-learning.xml, including the table of old
regex separators and what to pass now, and the description names the break. The
title uses the wording you suggested.
For now on, I'll create PRs from an apache branch. I'll try to minimize the
churn after an PR is created.
> RegEx removal part 7: BasicContextGenerator splits on the separator as a
> regular expression
> -------------------------------------------------------------------------------------------
>
> Key: OPENNLP-1929
> URL: https://issues.apache.org/jira/browse/OPENNLP-1929
> Project: OpenNLP
> Issue Type: Bug
> Components: Machine Learning
> Reporter: Kristian Rickert
> Assignee: Kristian Rickert
> Priority: Major
> Fix For: 3.0.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> {{BasicContextGenerator(String sep)}} documents its argument as a "separator
> character", but {{getContext}} calls {{String.split(separator)}}, which
> compiles the argument as a regular expression on every call.
> Consequences for a caller who follows the Javadoc:
> * {{"|"}}, {{"."}}, {{"+"}}, {{"*"}}, {{"?"}} split on every character or on
> nothing.
> * {{"("}} or {{"["}} throw {{PatternSyntaxException}} at prediction time, not
> at construction.
> * An empty separator splits the input into single characters.
> * If an application takes the separator from configuration, a crafted pattern
> is re-evaluated on every context string, which is a backtracking exposure.
> No code in the repository calls the separator constructor; it arrived with
> the maxent modularization (OPENNLP-1740) and has no test.
> Fix: split on the literal separator with a cursor scan, keeping
> {{String.split}} trailing-empty semantics; reject an empty separator with
> {{IllegalArgumentException}} at construction; state in the Javadoc that the
> separator is literal. Tests: default space, a metacharacter separator, a
> multi-character separator, leading and trailing separators, empty and null
> separator.
> Compatibility: external code that intentionally passes a pattern such as
> {{\s+}} relied on undocumented behavior; 3.0 is the release to correct it.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
