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

Deprecation warning in jsmin.php #416

Closed
chesio opened this issue Apr 5, 2024 · 7 comments
Closed

Deprecation warning in jsmin.php #416

chesio opened this issue Apr 5, 2024 · 7 comments

Comments

@chesio
Copy link
Contributor

chesio commented Apr 5, 2024

PHP Deprecated: ord(): Passing null to parameter #1 ($character) of type string is deprecated in /var/www/naturfuehrer/wordpress/wp-content/plugins/autoptimize/classes/external/php/jsmin.php on line 390

The reason seems to be that JSMin::get() method despite having @return string signature...

/**
* Return the next character from stdin. Watch out for lookahead. If the character is a control character,
* translate it to a space or linefeed.
*
* @return string
*/
protected function get()

...can actually return null as well:

if (ord($c) >= self::ORD_SPACE || $c === "\n" || $c === null) {
return $c;
}

EDIT: I was too quick... There is another path that returns null in this method (and triggers the warning above):

if ($c === null) {
// getc(stdin)
if ($this->inputIndex < $this->inputLength) {
$c = $this->input[$this->inputIndex];
$this->inputIndex += 1;
} else {
return $c;
}
}

@futtta futtta closed this as completed in c24b41f Apr 6, 2024
@futtta
Copy link
Owner

futtta commented Apr 6, 2024

can you confirm this fixes the deprecation warning @chesio ?

@futtta futtta reopened this Apr 6, 2024
@chesio
Copy link
Contributor Author

chesio commented Apr 8, 2024

can you confirm this fixes the deprecation warning @chesio ?

@futtta Unfortunately, it does not. This fix is incomplete, it has been reported in upstream repo as well: mrclay/jsmin-php#16

Would it make sense to update the whole library? It seems Autoptimize bundles version 2.3.2, while upstream has 2.4.3 as most recent: https://github.com/mrclay/jsmin-php/blob/master/CHANGELOG.md

@futtta futtta closed this as completed in 83852ba Apr 9, 2024
@futtta
Copy link
Owner

futtta commented Apr 9, 2024

made a small tweak to the current version, can you re-test @chesio

some of the changes in 2.4.3 are already in "my" version (which is different from the original 2.3.2), but it might indeed be time to go to the latest JSmin version. :-)

@futtta futtta reopened this Apr 9, 2024
@chesio
Copy link
Contributor Author

chesio commented Apr 9, 2024

@futtta I apologize, I should have been more explicit in my previous comments.

While the patch prevents one possible source of the deprecation message it does not fix the problem I reported, because JSMin::get() can still return null. The deprecation notice I am seeing is triggered here:

$get = $this->get();
$comment .= $get;
if (ord($get) <= self::ORD_LF) { // end of line reached

On line 388 JSMin::get() returns null, on line 390 this null value is passed to ord().

As far as I can tell, this has been fixed upstream in mrclay/jsmin-php@a688631 The fix does not prevent null being returned from JSMin::get(), but fixes all code handling the returned value to expect null as well.

@futtta
Copy link
Owner

futtta commented Apr 9, 2024

arghhhh ... no use in trying put band-aids on the old code, I'll have to bite the bullet. more soon :)

@futtta futtta closed this as completed in 1605c57 May 9, 2024
@futtta
Copy link
Owner

futtta commented May 9, 2024

reopening so we can test ;-)

@chesio had to make some small tweaks, but this is 99% of jsmin.php, can you test to confirm this fixes the issues you were facing?

@futtta futtta reopened this May 9, 2024
@chesio
Copy link
Contributor Author

chesio commented May 17, 2024

@futtta Sorry for keeping you waiting so long. I have now tested the beta branch and I don't get the deprecation warnings anymore! 👍

@futtta futtta closed this as completed Aug 11, 2024
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

No branches or pull requests

2 participants