Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CODEC-312: Fix possible StringIndexOutOfBoundException thrown by MatchRatingApproachEncoder.encode() method #220

Merged

Conversation

arthurscchan
Copy link
Contributor

@arthurscchan arthurscchan commented Nov 22, 2023

This fixes a possible StringIndexOutOfBoundException in src/main/java/org/apache/commons/codec/language/MatchRatingApproachEncoder.java thrown by MatchRatingApproachEncoder.encode() method when the input string only contains punctuations or vowels.

This PR adds some conditional checking to ensure the string is not empty after each method call. If it is empty after any method call, it will simply return EMPTY and avoid continuing processing onto the next processing method.

We found this bug using fuzzing by way of OSS-Fuzz. It is reported at https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=64359.

@garydgregory
Copy link
Member

Hello @arthurscchan
Same comment: You'll need a unit test to prove what the main code does.

@arthurscchan
Copy link
Contributor Author

Hi, I have added the unit test.

@garydgregory
Copy link
Member

@arthurscchan
Please use a better description in PRs and JIRA: Specify the class and method where the exception occurs.

Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arthurscchan
See my comments, and also, comments in other PRs that apply here.

Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @arthurscchan
Please see my comments here and in other PRs that apply here.

@arthurscchan arthurscchan changed the title CODEC-312: Fix possible StringIndexOutOfBoundException CODEC-312: Fix possible StringIndexOutOfBoundException throw by MatchRatingApproachEncoder.encode() method Nov 24, 2023
@arthurscchan arthurscchan changed the title CODEC-312: Fix possible StringIndexOutOfBoundException throw by MatchRatingApproachEncoder.encode() method CODEC-312: Fix possible StringIndexOutOfBoundException thrown by MatchRatingApproachEncoder.encode() method Nov 24, 2023
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @arthurscchan
Please see my comments.

Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (44e4c4d) 92.27% compared to head (f616911) 92.22%.
Report is 6 commits behind head on master.

Files Patch % Lines
...ons/codec/language/MatchRatingApproachEncoder.java 25.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #220      +/-   ##
============================================
- Coverage     92.27%   92.22%   -0.06%     
- Complexity     1742     1745       +3     
============================================
  Files            67       67              
  Lines          4584     4591       +7     
  Branches        709      713       +4     
============================================
+ Hits           4230     4234       +4     
- Misses          242      243       +1     
- Partials        112      114       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@garydgregory garydgregory merged commit 047d247 into apache:master Nov 25, 2023
8 checks passed
garydgregory added a commit that referenced this pull request Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants