More PHP 8 deprecation warnings

[Chris Filkins]

Deprecated: strrpos(): Passing null to parameter #2 ($needle) of type string is deprecated in /var/www/html/vendor/tsugi/lib/src/Util/U.php on line 415
Array ( [type] => 8192 [message] => strrpos(): Passing null to parameter #2 ($needle) of type string is deprecated [file] => /var/www/html/vendor/tsugi/lib/src/Util/U.php [line] => 415 ) 

As I'm preparing to migrate our production environment over to PHP 8.2.12, I'm seeing these deprecated warnings on every launch.  I will try to take a look at how it's being used, but thought I'd put this out there in case it's already been talked about.

[Chris Filkins]

Just to expand, I'm seeing this on our production servers, because we don't use the $apphome config variable, so it's passing in a null value in oidc_launch...

if (!U::startsWith($launch_url, $CFG->apphome)) {
    LTIX::abort_with_error_log("Launch_url must start with " . $CFG->apphome);
}

--
You received this message because you are subscribed to the Google Groups "Tsugi Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to tsugi-dev+...@apereo.org.
To view this discussion on the web visit https://groups.google.com/a/apereo.org/d/msgid/tsugi-dev/a93e0121-0afa-430e-930e-4af6f22d6650n%40apereo.org.

[Chuck Severance]

Chris - this is an easy fix.  Expect a fix by the end of today.

/Chuck

To view this discussion on the web visit https://groups.google.com/a/apereo.org/d/msgid/tsugi-dev/CAN4-RNhz8Z4r9g9XkZoCxUS5VVy3XHxscoz%3DnnNzXzrHW293MQ%40mail.gmail.com.

[Chris Filkins]

Awesome, thanks! 

[Charles Severance]

All fixed.

https://github.com/tsugiproject/tsugi/commit/79623846409e0483cd6877886901b5119afe6830

I even extended the unit tests to break and then fixed the code so the unit tests passed. 

Made me feel like a real professional programmer.

Thanks.

/Chuck

To view this discussion on the web visit https://groups.google.com/a/apereo.org/d/msgid/tsugi-dev/CAN4-RNhD-o5hTCJjMJFzDYU1qJ2k06SPOFpQ-xqmRz%2Bh2oBxWQ%40mail.gmail.com.

[Chris Filkins]

I  just had a chance to circle back to this, and I think perhaps it's backwards...should it return TRUE instead of FALSE?

My issue is in oidc_launch.php:

if (!U::startsWith($launch_url, $CFG->apphome)) {
    LTIX::abort_with_error_log("Launch_url must start with " . $CFG->apphome);
}

We don't have apphome configured (our prod servers), so returning FALSE here actually prevents the launch.  I put this into testing and found that I'd get the "Launch_url must start with " message :(  Or would it be best to update oidc_launch to validate that apphome is set first?

-Chris

[Chris Filkins]

...Or I can make sure that I fully understand the config file for Tsugi.  We were using $apphome = false, misunderstanding the comments.  In testing, I was able to reproduce the issue on my local instance, and we'll be updating the config file on production next week.  My apologies for the confusion :(

-Chris

[Charles Severance]

Chris,

You said you figured this out.   But I don’t understand.

This looks like a bug to me.  Setting apphome should *not* be required - but since I never use it that way in my servers - the code paths are rarely tested.

I want to fix this - but want to understand what you are doing to work around it.

/Chuck

To view this discussion on the web visit https://groups.google.com/a/apereo.org/d/msgid/tsugi-dev/CAN4-RNgOQ409OVdPEB7xe%3DgXn7cbruScSQnegVwGMbO9w%3DNx3w%40mail.gmail.com.

[Chris Filkins]

What I found to be our underlying issue is that we had not defined the $apphome variable in our config.php file at all, and only defined $wwwroot.  So the critical functions were working fine when they'd fall back to $wwwroot, but the handful of things that relied on $apphome were giving us some headaches. 

-Chris

[Charles Severance]

So I want to find and fix all those things.  It should fall back to wwwroot - you can send in a PR on anything you find.   I will fix this the one you found.

/Chuck

[Charles Severance]

Chris - This is now fixed in master and in production in most of my servers.

/Chuck

To view this discussion on the web visit https://groups.google.com/a/apereo.org/d/msgid/tsugi-dev/CAN4-RNgOQ409OVdPEB7xe%3DgXn7cbruScSQnegVwGMbO9w%3DNx3w%40mail.gmail.com.

[Chris Filkins]

And just to clarify...best practices, should we have set both $apphome and $wwwroot, or just $wwwroot?  These are dedicated servers that do nothing but Tsugi.  Thanks!

-Chris

[Charles Severance]

On Mar 26, 2024, at 9:29 AM, Chris Filkins <cfil...@gmail.com> wrote:

And just to clarify...best practices, should we have set both $apphome and $wwwroot, or just $wwwroot?  These are dedicated servers that do nothing but Tsugi.  Thanks!

-Chris

The design hals always been that apphome is optional.  The default of null is supposed to work.  If you are using Tsugi as a library / app store only with no “front page” you set wwwroot and leave apphome null.  Back in ancient Tsugi and pre-Tsugi history, there was only wwwroot :)

The little bugs you are encountering are *not* the code passive aggressively pushing you to set apphome :)  They are just bugs that you encounter because none of my 20+ servers leave apphome as null - so I never exercise code paths with apphome as null - so you find them.

And I want you to find them and fix them.   The code is always supposed to fall back to wwwroot.  Of course the PHP 8.2 obsession with "nulls are not empty strings” exposed a few of these :)   But those are easy to fix and need to be fixed.

Thanks.

/Chuck

[Chris Filkins]

Sounds good!  Just wanted to make sure I wasn't barking up the wrong tree :)

-Chris