Skip to content

Fix StringService edge cases: mask, isPalindrome, random, truncate and more - #393

Merged
techmahedy merged 2 commits into
doppar:4.xfrom
techmahedy:fix/string-mask-zero-visible-end
Oct 30, 2026
Merged

techmahedy merged 2 commits into
doppar:4.xfrom
techmahedy:fix/string-mask-zero-visible-end

Conversation

@techmahedy

Copy link
Copy Markdown
Member

Pull Request Checklist

Q A
Branch? 4.x
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

Description

A review of StringService found several methods that return wrong results or unsafe output on edge-case input. Most of them count bytes where the rest of the class is Unicode-aware, and a few have logic errors.

The most serious one: mask('1234567890', 3, 0) returned 123*******1234567890, so the whole value was exposed. substr($string, -0) is substr($string, 0), which returns the entire string.

Fixes

Method Problem Fix
mask A zero visible end returned the whole secret. It also counted bytes. Handles a zero visible end and counts characters. Negative counts are treated as 0.
isPalindrome 'привет' returned true, because non-ASCII letters were stripped. Unicode-aware.
random Used str_shuffle (not cryptographically secure). A negative length threw a ValueError. Uses random_int. A negative length returns ''.
uuid Used mt_rand. Uses random_bytes, with the v4 version and variant bits set.
truncate Counted bytes, so it could cut a character in half. Counts characters. A negative max is clamped to 0.
snake snake('ÉcoleNom') gave École_nom and snake('éNom') gave énom. Unicode-aware. ASCII behavior is unchanged.
slug An empty separator emitted PHP warnings and returned ''. Returns the joined words.
contains An empty needle matched, unlike startsWith and endsWith, which skip it. An empty needle never matches.
limitWords A negative limit gave odd output, and invalid UTF-8 threw a TypeError. The limit is clamped to 0, with a byte-wise fallback for invalid UTF-8.
longestCommonSubstring Could return half a character. It also built a full n×m matrix. Works on characters with a rolling row.
highlightKeyword A keyword of '0' was treated as empty. Case-insensitive matching ignored non-ASCII. Fixed both. Invalid UTF-8 input is still highlighted byte-wise.
toUpper toUpper('éa') gave éA. Uses mb_strtoupper.
suffixAppend Called the global str() helper, so it threw outside a booted container. It crashed on ''. Uses $this->endsWith(). An empty input returns the suffix.
removeSuffix removeSuffix('hello', 'xyz') returned he, and an empty suffix returned ''. Removes only a suffix the string really ends with.

PHPStan baseline

phpstan-baseline.neon allows exactly 3 "undefined property" errors each for Builder::$attributes and Builder::$originalAttributes. The insert fix in #392 added a fourth of each, so PHPStan was failing on 4.x. The trait is shared by Model and Builder, and only Model declares those properties. This PR raises both counts from 3 to 4. It is unrelated to StringService, and can be split into its own PR if you prefer.

Backward Compatibility

  • Output changes only for input the old code got wrong: Unicode text, zero or negative lengths, empty arguments, and a non-matching removeSuffix.
  • snake for ASCII input is unchanged, so table names and computed-property keys stay the same.
  • contains('x', '') now returns false instead of true.
  • random() and uuid() keep their formats, and now use a cryptographically secure source.

Checklist

  • Tests have been added or updated
  • Documentation has been updated if necessary
  • Code follows the project coding standards
  • All tests pass locally

@techmahedy techmahedy added the bug Something isn't working label Oct 30, 2026
@techmahedy
techmahedy merged commit 0947f3b into doppar:4.x Oct 30, 2026
13 checks passed
@techmahedy
techmahedy deleted the fix/string-mask-zero-visible-end branch October 30, 2026 16:04
@techmahedy
techmahedy restored the fix/string-mask-zero-visible-end branch October 30, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant