SimpleMachines / SMF

Simple Machines Forum — SMF in short — is free and open-source community forum software, delivering professional grade features in a package that allows you to set up your own online community within minutes!
https://www.simplemachines.org/
Other
592 stars 255 forks source link

[3.0]: Config quirks #8293

Open sbulen opened 3 months ago

sbulen commented 3 months ago

Basic Information

  1. In Windows, Settings.php is being reloaded constantly, mainly due to it's not being found in a get_included_files() check. This is due to the different DIRECTORY_SEPARATORs between Windows & linux.
  2. eval returns null, which can result in null being assigned to a setting under some circumstances. E.g., attempting an upgrade, received the following error:

image

Steps to reproduce

  1. Install 2.1.4
  2. Upgrade to 3.0

Expected result

A 3.0 forum

Actual result

WSOD error

Version/Git revision

4.0 Alpha 2 - current GH

Database Engine

All

Database Version

8.4.0

PHP Version

8.3.8

Logs

Fatal error: Uncaught TypeError: Cannot assign null to property SMF\Config::$languagesdir of type string in D:\wamp64\www\84van30\Sources\Config.php:915 Stack trace: #0 D:\wamp64\www\84van30\Sources\Config.php(886): SMF\Config::set(Array) #1 D:\wamp64\www\84van30\upgrade.php(188): SMF\Config::load() #2 {main} thrown in D:\wamp64\www\84van30\Sources\Config.php on line 915

Additional Information

No response

sbulen commented 3 months ago

I'm using my WAMP...

I think at least part of the problem is directory separators... It keeps thinking things aren't loaded... image

sbulen commented 3 months ago

I believe the rest of the issue is that eval() is used in Config.php against the definition default value strings. But... eval() RETURNS NULL unless there is an explicit return in the eval'd code...
https://github.com/SimpleMachines/SMF/blob/7788cb5c26ab7bc7ea77d3fac6ec2b29bef3f854/Sources/Config.php#L915

Using double quotes, ", will likely work better.

sbulen commented 3 months ago

Lots of options on the DIRECTORY_SEPARATOR question...

Option 1: Bite the bullet & use DIRECTORY SEPARATOR consistently throughout; never hardcode '/' again... Ensure folder settings are OS-appropriate.

Proper solution. Least fun. Like having a paladin on the team.

Option 2: At runtime, normalize important paths to unix '/', but the OS-sensitive functions, e.g., get_included_files(), won't work. We'd have to write our own wrappers around them. Note that Windows honors '/' in paths, but uses '\' in return calls like get_included_files(). Least effort, feels almost clean. Most consistent with past SMF behavior.

Option 3: At runtime, normalize important paths to DIRECTORY_SEPARATOR so they work better with OS-sensitive functions. Feels a bit more hackish, though to be honest, likely cleaner than option 2.

Implications here with repair_settings.php also. Likely other utilities.

sbulen commented 3 months ago

I've confirmed the following works to fix the eval issue: self::${$var} = eval('return ' . "$default;");

jdarwood007 commented 3 months ago

In Windows, Settings.php is being reloaded constantly, mainly due to it's not being found in a get_included_files() check. This is due to the different DIRECTORY_SEPARATORs between Windows & linux.

Should we stop using the hard-coded / and replace all instances with DIRECTORY_SEPARATOR?

Sesquipedalian commented 3 months ago

Should we stop using the hard-coded / and replace all instances with DIRECTORY_SEPARATOR?

Yes, we should.

sbulen commented 3 months ago

So it's confirmed we have a paladin on the team.

Now, or after the installer/upgrader rewrite gets merged?

Sesquipedalian commented 3 months ago

After.

live627 commented 3 months ago

I'm in favor of option 3, above, and was under the impression that that's always been the way it should be done, probably combined with option 2 because it needed normalised, at least when I rewrote the hooks management system

On Thu, Jul 18, 2024 at 6:13 PM Jon Stovell @.***> wrote:

After.

— Reply to this email directly, view it on GitHub https://github.com/SimpleMachines/SMF/issues/8293#issuecomment-2237836178, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADJNNYQ5UMXX2WWM3GS62DZNBR4LAVCNFSM6AAAAABK7YVNQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZXHAZTMMJXHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>