@diosmosis opened this Pull Request on May 21st 2021 Member

Description:

Fixes #17583

The string definition passed into Segment will sometimes be decoded before being saved in Segment::$string. So if we reuse the string to create a new Segment, the new segment's hash will be different. This PR changes getString() to return the original string so we're always using the definition we started with.

EDIT: Modified this PR to not change Segment::getString(), since that resulted in a test failure that looked like a regression. Instead there's a new method. We can replace every occurrence if desired in 4.4.0.

Review

  • [ ] Functional review done
  • [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@diosmosis commented on May 21st 2021 Member

An alternative solution here is just to put the urlencode() back in.

This Pull Request was closed on May 23rd 2021
Powered by GitHub Issue Mirror