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.

Sign up for GitHub

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

Jump to bottom

"Illegal string offset" PHP warnings #642

Closed
hannob opened this issue Oct 2, 2019 · 9 comments
Closed

"Illegal string offset" PHP warnings #642

hannob opened this issue Oct 2, 2019 · 9 comments
Labels
bugs
Milestone
2.3.2

Comments

@hannob
Copy link
Contributor

hannob commented Oct 2, 2019

Accessing certain URLs gives me plenty of "Illegal string offset" warnings in the PHP log.

I reproduced this on a fresh installation, so I'm reasonably confident it's nothing with special extensions or special config options. This happens in all PHP versions from 7.1 to 7.4 (i.e. all currently supported ones), I haven't tested older ones.

The errors get triggered when an URL of this form is accessed:

https://[host]/index.php?url=archives/1&serendipity=true

The warnings in the log look like this:

[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'action' in [path]/serendipity_config.inc.php on line 458
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Cannot assign an empty string to a string offset in [path]/serendipity_config.inc.php on line 458
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'adminAction' in [path]/serendipity_config.inc.php on line 462
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Cannot assign an empty string to a string offset in [path]/serendipity_config.inc.php on line 462
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'action' in [path]/include/functions_routing.inc.php on line 11
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'action' in [path]/include/genpage.inc.php on line 37
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'category' in [path]/include/functions_entries.inc.php on line 213
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'hide_category' in [path]/include/functions_entries.inc.php on line 213
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'viewAuthor' in [path]/include/functions_entries.inc.php on line 213
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'page' in [path]/include/functions_entries.inc.php on line 213
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'adminModule' in [path]/include/functions_entries.inc.php on line 213
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'cview' in [path]/include/functions_entries.inc.php on line 1177
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'action' in [path]/include/functions_entries.inc.php on line 960
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'page' in [path]/include/functions_entries.inc.php on line 961
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'page' in [path]/include/functions_entries.inc.php on line 966
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'page' in [path]/include/functions_entries.inc.php on line 976
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'page' in [path]/include/functions_entries.inc.php on line 978
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'page' in [path]/include/functions_entries.inc.php on line 980
[02-Oct-2019 15:33:53 Europe/Berlin] PHP Warning:  Illegal string offset 'action' in [path]/include/genpage.inc.php on line 133
@hannob
Copy link
Contributor Author

hannob commented Oct 2, 2019

Forgot: Tested with current release version (2.3.1).

@onli onli added the bugs label Oct 2, 2019
@fe-hicking
Copy link
Contributor

Serendipity is currently not compatible with PHP E_NOTICE turned on.

Usually production versions of s9y should turn off E_NOTICE, and only alpha/testing versions should have it on. I think there are circumstances, where a PHP error_reporting is not touched, if it is specifically set on an instance.

Having said that, it would be much appreciated if Serendipity could take care of all uninitialized variables. However because it stemming from times where dynamic typing was still a valid and easy thing to do, it is much work to initialize each and every variable before use or check for its existance, which will also blow up the amount of code.

@hannob
Copy link
Contributor Author

hannob commented Oct 6, 2019

These aren't E_NOTICE warnings.

Test:

$ php -r 'error_reporting(E_ALL&~E_NOTICE);$a="";$a["x"];'
PHP Warning:  Illegal string offset 'x' in Command line code on line 1

garvinhicking pushed a commit that referenced this issue Oct 7, 2019
@fe-hicking
[TASK] Prevents PHP warnings with type differences on $_REQUEST['sere…
e8b8be0 
…ndipity']

refs #642
@garvinhicking
Copy link
Member

Very true. Thanks for pointing out my mistake. I just pushed a commit that should catch this type misbehaviour . The patch should also apply to older s9y releases, would you be able to confirm this fixes the warnings?

th-h pushed a commit to th-h/Serendipity that referenced this issue Oct 13, 2019
@fe-hicking @th-h
[TASK] Prevents PHP warnings with type differences on $_REQUEST['sere…
804f9f1 
…ndipity']

refs s9y#642
@th-h th-h added this to the 2.3.2 milestone Oct 13, 2019
th-h pushed a commit to th-h/Serendipity that referenced this issue Oct 13, 2019
@fe-hicking @th-h
[TASK] Prevents PHP warnings with type differences on $_REQUEST['sere…
2b96162 
…ndipity'].

refs s9y#642

Backported from master branch.

Signed-off-by: Thomas Hochstein <thh@inter.net>
@hannob
Copy link
Contributor Author

hannob commented Oct 13, 2019

This fixes the warnings, but it introduces more PHP notices due to accessing an undefined index.

This can easily be fixed with an array_key_exists check, I'll do a pull request.

hannob added a commit to hannob/Serendipity that referenced this issue Oct 13, 2019
@hannob
Avoid undefined index notices, see s9y#642
5765d1a
@hannob hannob mentioned this issue Oct 13, 2019
Merged
th-h pushed a commit to th-h/Serendipity that referenced this issue Oct 13, 2019
@hannob @th-h
Avoid undefined index notices.
4a86162 
See s9y#642.

Backported from master branch.

Signed-off-by: Thomas Hochstein <thh@inter.net>
garvinhicking pushed a commit that referenced this issue Oct 15, 2019
@fe-hicking
[BUGFIX] Fixes missing referenced variable when $_GET['serendipity'] …
5b35569 
…is not an array

refs #642
refs #653
@garvinhicking
Copy link
Member

@hannob I saw a major problem; in the code we actually assign $_GET['serendipity']['action'] = 'read', but the new code initializes an empty, non-referenced array for that case.

I hope the new commit properly addresses both your concern and allows proper function, could you test that on your site if it still prevents undefined index notices?

@garvinhicking
Copy link
Member

(See #653 for reference)

@garvinhicking garvinhicking mentioned this issue Oct 15, 2019
Closed
@hannob
Copy link
Contributor Author

hannob commented Oct 15, 2019

oh yeah, sorry for my insufficient testing.

I can confirm the bug, I applied the patch and will look if new warnings appear in the logs.

th-h pushed a commit to th-h/Serendipity that referenced this issue Oct 15, 2019
@fe-hicking @th-h
[BUGFIX] Fixes missing referenced variable when $_GET['serendipity'] …
2ea8863 
…is not an array

refs s9y#642
refs s9y#653
@hannob
Copy link
Contributor Author

hannob commented Dec 14, 2019

I think we're done here.

@hannob hannob closed this as completed Dec 14, 2019
robelix pushed a commit to robelix/Serendipity that referenced this issue Mar 9, 2020
@fe-hicking @robelix
[TASK] Prevents PHP warnings with type differences on $_REQUEST['sere…
6388f3d 
…ndipity']

refs s9y#642
robelix pushed a commit to robelix/Serendipity that referenced this issue Mar 9, 2020
@hannob @robelix
Avoid undefined index notices, see s9y#642
b66c56a
robelix pushed a commit to robelix/Serendipity that referenced this issue Mar 9, 2020
@fe-hicking @robelix
[BUGFIX] Fixes missing referenced variable when $_GET['serendipity'] …
1d1653a 
…is not an array

refs s9y#642
refs s9y#653
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bugs
Projects
None yet
Milestone
2.3.2
Development

No branches or pull requests
5 participants
@garvinhicking @th-h @onli @hannob @fe-hicking