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

[3.0]: Check for "calendar_holidays" table during upgrade could return false positives #8261

Open
Oldiesmann opened this issue Jun 27, 2024 · 1 comment · May be fixed by #8093
Open

[3.0]: Check for "calendar_holidays" table during upgrade could return false positives #8261

Oldiesmann opened this issue Jun 27, 2024 · 1 comment · May be fixed by #8093

Comments

@Oldiesmann
Copy link
Contributor

Basic Information

The check to see if a "calendar_holidays" table exists prior to attempting to convert holidays to the new events format could return a false positive if there are old backup tables, another (older) SMF installation on the same database or something else using the same database that happens to have a table with a name ending in "calendar_holidays". This will lead to an error on the next step since SMF will be trying to pull from {db_prefix}calendar_holidays when the table doesn't exist

Steps to reproduce

  1. Run the upgrader when the database that SMF is using doesn't have the "{prefix}calendar_holidays" table but has another table with a name ending in "calendar_holidays" (such as "backup_calendar_holidays")

Expected result

Code should determine that the specific table it's looking for isn't there

Actual result

Code checks for ''any'' table ending in "calendar_holidays", causing an error if there are old backup tables or another table matching "%calendar_holidays".

Version/Git revision

3.0 Alpha 1

Database Engine

All

Database Version

10.11.8-MariaDB-ubu2204

PHP Version

8.3.8

Logs

No response

Additional Information

Filing this instead of submitting a PR because I can't tell if the db_list_tables method supports the standard {db_prefix} replacement syntax or not.

@Sesquipedalian
Copy link
Member

Sesquipedalian commented Jun 27, 2024

I've added a fix for this in #8093.

@Sesquipedalian Sesquipedalian linked a pull request Jun 27, 2024 that will close this issue
16 tasks
@Sesquipedalian Sesquipedalian added this to the 3.0 Alpha 3 milestone Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants