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 #4374 prevent PHP fatal error by adding guard clauses if preg_replace() fails in delay JS add script #4414

Merged
merged 1 commit into from
Oct 8, 2021

Conversation

Tabrisrp
Copy link
Contributor

@Tabrisrp Tabrisrp commented Oct 7, 2021

Description

Add guard clauses if preg_replace() fails in add_delay_js_script(), to avoid passing a null value to move_meta_charset_to_head() instead of a string.

Fixes #4374

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Is the solution different from the one proposed during the grooming?

No grooming done

How Has This Been Tested?

  • Automated tests

Checklist:

Please delete the options that are not relevant.

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Existing unit tests pass locally with my changes

@Tabrisrp Tabrisrp added type: bug Indicates an unexpected problem or unintended behavior module: delay JS labels Oct 7, 2021
@Tabrisrp Tabrisrp added this to the 3.10.1 milestone Oct 7, 2021
@Tabrisrp Tabrisrp requested a review from a team October 7, 2021 15:12
@Tabrisrp Tabrisrp self-assigned this Oct 7, 2021
@Tabrisrp Tabrisrp requested a review from a team October 8, 2021 15:39
@piotrbak
Copy link
Contributor

piotrbak commented Oct 8, 2021

@Tabrisrp Are we 100% sure that $html won't be null here?

Shall we do any check there before assigning the value? Assigning null there causes fatal error.

@Tabrisrp
Copy link
Contributor Author

Tabrisrp commented Oct 8, 2021

@piotrbak Yes at this point $html should always be a string

Copy link
Contributor

@piotrbak piotrbak left a comment

Choose a reason for hiding this comment

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

Ready to go then!
testrail-report-272.pdf

@Tabrisrp Tabrisrp merged commit 5ad8bf6 into develop Oct 8, 2021
@Tabrisrp Tabrisrp deleted the fix/4374-delay-js-null branch October 8, 2021 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: delay JS needs: qa type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP Fatal error: preg_match() expects parameter 2 to be string, null given in /DelayJS/HTML.php:221
3 participants