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

Fixes passing null to strlen() inside escapeHtml() #2884

Merged
merged 1 commit into from
Jan 3, 2023
Merged

Fixes passing null to strlen() inside escapeHtml() #2884

merged 1 commit into from
Jan 3, 2023

Conversation

sreichel
Copy link
Contributor

@sreichel sreichel commented Jan 3, 2023

Description (*)

I don't disagree with fixing existing uses of escapeHtml but just for practicality sake this is going to be a big headache for those with lots of extensions so I propose changing the signature of escapeHtml to allow a nullable string (if null is passed it should return an empty string).

We just need to check if we pass as string to strlen() inside escapeHtml(). Should be enough?

Related Pull Requests

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@sreichel sreichel added the PHP 8.1 Related to PHP 8.1 label Jan 3, 2023
@github-actions github-actions bot added the Component: Core Relates to Mage_Core label Jan 3, 2023
@sreichel sreichel changed the title Fixes passing null to escapeHtml() Fixes passing null to strlen() inside escapeHtml() Jan 3, 2023
@Flyingmana Flyingmana merged commit 829daf0 into OpenMage:1.9.4.x Jan 3, 2023
@sreichel
Copy link
Contributor Author

sreichel commented Jan 3, 2023

Okay ... to late to change it to $data !== ''.

@sreichel sreichel deleted the fix/php8.1/escapeHtml-null branch January 3, 2023 20:36
@sreichel
Copy link
Contributor Author

sreichel commented Jan 3, 2023

@Flyingmana @fballiano where is the project tab? I've used that to track some issues. Recently added "PHP8.1"

@fballiano
Copy link
Contributor

My fault, was trying to hide the double “projects/projects classic” and they both got hidden, but now it’s ok and there’s only one projects tab

@colinmollenhour
Copy link
Member

Thanks, @sreichel !

sreichel added a commit to elidrissidev/magento-lts that referenced this pull request Jan 3, 2023
Reverted changes to escapeHtml() ... see OpenMage#2884
sreichel added a commit to elidrissidev/magento-lts that referenced this pull request Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Relates to Mage_Core PHP 8.1 Related to PHP 8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants