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

FixedLengthTruncator throws exception in some cases #1546

Open
bambuca opened this issue Oct 2, 2024 · 2 comments · May be fixed by #1547
Open

FixedLengthTruncator throws exception in some cases #1546

bambuca opened this issue Oct 2, 2024 · 2 comments · May be fixed by #1547

Comments

@bambuca
Copy link

bambuca commented Oct 2, 2024

"Short".Truncate(100, null); will throw exception:

In the Truncate method this code casues error, because value is shorter than lenght and thus value.Substring(0, legth) throws exception

https://github.com/Humanizr/Humanizer/blob/main/src/Humanizer/Truncation/FixedLengthTruncator.cs , line 21-26
if (truncationString == null || truncationString.Length > length)
{
return truncateFrom == TruncateFrom.Right
? value.Substring(0, length)
: value.Substring(value.Length - length);
}

@hangy
Copy link
Contributor

hangy commented Oct 2, 2024

Do you have a short repro for this? "Short".Truncate(100, null) doesn't even compile for me, as the overloads are ambiguous. "Short".Truncate(100) correctly returns "Short". There are actually test cases for a case when the input string is shorter than the length.

@bambuca bambuca linked a pull request Oct 2, 2024 that will close this issue
11 tasks
@bambuca
Copy link
Author

bambuca commented Oct 2, 2024

Sorry, my bad. The null parameter should be (string)null. See here https://dotnetfiddle.net/qZcZ7f

In FixedLengthTruncator.cs, you correctly check if (value.Length > length) in most cases. However, this check is missing when truncationString == null. In that case, you call value.Substring(0, length), which can fail if value is shorter than the specified length.

I crated PR for that #1547

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants