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

Fix fatal error in PHP8.x for invalid get data #1754

Merged
merged 12 commits into from Jan 24, 2024

Conversation

amieiro
Copy link
Member

@amieiro amieiro commented Jan 4, 2024

Problem

A malformed sort array throws a fatal error in PHP 8.x (in PHP 7.4 throws a warning).

Before this fix, if you add ?sort[by][foobar]=1 to a GlotPress URL, in a GET request, you will get an error 500.

Before

wget -S https://glotpress.test/glotpress/projects/local-plugins/glotpress/gl/default/\?sort\[by\]\[foobar\]\=1
...
HTTP request sent, awaiting response...
  HTTP/1.1 500 Internal Server Error

After

wget -S https://glotpress.test/glotpress/projects/local-plugins/glotpress/gl/default/\?sort\[by\]\[foobar\]\=1
...
HTTP request sent, awaiting response...
  HTTP/1.1 200 OK

Fixes #1753.

Solution

The malformed URL throws an error at wp-content/plugins/GlotPress/gp-includes/misc.php on line 66 because the $key parameter in the gp_array_get() function is an array. This PR checks if the $key is a string or a number before using it as key in the array.

After solving this first problem, it arises another warning:

PHP Warning:  Array to string conversion in wp-content/plugins/GlotPress/gp-templates/translations.php on line 135
PHP Warning:  Array to string conversion in wp-content/plugins/GlotPress/gp-templates/translations.php on line 149
PHP Warning:  Array to string conversion in wp-content/plugins/GlotPress/gp-templates/translations.php on line 163
PHP Warning:  Array to string conversion in wp-content/plugins/GlotPress/gp-templates/translations.php on line 177
PHP Warning:  Array to string conversion in wp-content/plugins/GlotPress/gp-templates/translations.php on line 191
PHP Warning:  Array to string conversion in wp-content/plugins/GlotPress/gp-templates/translations.php on line 207
PHP Warning:  Array to string conversion in wp-content/plugins/GlotPress/gp-templates/translations.php on line 221

This problem is solved removing the values from the filters that are not strings or numbers.

@amieiro amieiro marked this pull request as ready for review January 4, 2024 15:49
dd32
dd32 previously approved these changes Jan 5, 2024
gp-includes/misc.php Outdated Show resolved Hide resolved
gp-templates/translations.php Outdated Show resolved Hide resolved
Co-authored-by: Dion Hulse <dd32@dd32.id.au>
Co-authored-by: Dion Hulse <dd32@dd32.id.au>
@amieiro amieiro requested a review from dd32 January 5, 2024 15:57
gp-includes/misc.php Outdated Show resolved Hide resolved
@trymebytes trymebytes self-requested a review January 24, 2024 10:56
trymebytes
trymebytes previously approved these changes Jan 24, 2024
Copy link
Contributor

@trymebytes trymebytes left a comment

Choose a reason for hiding this comment

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

I left a tiny comment but looks good to me!

@amieiro amieiro merged commit 98e5f2f into GlotPress:develop Jan 24, 2024
11 checks passed
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 this pull request may close these issues.

Warning (PHP7) / Fatal (PHP8) triggered by user inputs in gp_array_get()
3 participants