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

null on trim #824

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

lazardanlucian
Copy link

We have some cases where our pages in wp-admin don't work.

Fatal error: Uncaught Error: trim(): Argument #1 ($string) must be of type string, null given in /var/www/wp-content/mu-plugins/query-monitor/collectors/raw_request.php on line 73

Fatal error: Uncaught Error: trim(): Argument johnbillion#1 ($string) must be of type string, null given
in /var/www/wp-content/mu-plugins/query-monitor/collectors/raw_request.php on line 73
@johnbillion
Copy link
Owner

johnbillion commented Oct 25, 2023

Thanks for the PR. It sounds like your server is outputting an HTTP header in an unexpected format. Can you provide a list of the headers in the response so we can see what's up?

@lazardanlucian
Copy link
Author

Hello, I found the issue.
There was a header set without a value, from a php snippets plugin.

This still could be useful here, since a header without a value will not break anything, why have it break qm

@lazardanlucian
Copy link
Author

array (size=3)
0 => string 'X-Powered-By: PHP/8.0.27' (length=24)
1 => string 'Sample-Header-No-Value' (length=22)
2 => string 'Test: Value' (length=11)

this will break QM and wp-admin pages where Sample header exists

@lazardanlucian
Copy link
Author

lazardanlucian commented Oct 25, 2023

@johnbillion Although the RFC mandate a header value, I'm not sure if the application should break if there is a header without a value.

So our problem is fixed by fixing the header, so it's up to you how you want to treat this 🤖

@johnbillion
Copy link
Owner

Thanks, I'll make some adjustments to the header handling to handle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants