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.

Sign up for GitHub

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

Jump to bottom

[4.1] Fix generate web cron key #37868

Merged
merged 2 commits into from
Jun 5, 2022
Merged

[4.1] Fix generate web cron key #37868

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4e53c0
Update schedulerunner.php
joomdonation May 23, 2022
b09ee6b
Code simplify
joomdonation May 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions plugins/system/schedulerunner/schedulerunner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,7 @@ public function generateWebcronKey(EventInterface $event): void
/** @var Extension $table */
[$context, $table] = $event->getArguments();

if ($context !== 'com_config.component'
|| ($table->name ?? '') !== 'COM_SCHEDULER')
if ($context !== 'com_config.component' || $table->name !== 'com_scheduler')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do a strtolower on this please - covers both cases with minimal overhead. If this used to work - it's safer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wilsonge We can but is it really needed? The install and update SQL, from my quick check, is just com_scheduler. Name of all of our components in extensions table is also lower case, so I don't see the need for that. Strange that this feature worked before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might depend on the collation if a string comparison in MySQl is case sensitive or not. See e.g. https://stackoverflow.com/questions/5629111/how-can-i-make-sql-case-sensitive-string-comparison-on-mysql or https://stackoverflow.com/questions/5629111/how-can-i-make-sql-case-sensitive-string-comparison-on-mysql . So for people using the Joomla standard "utf8mb4_unicode_ci" it might have worked, and for people using some collation ending with "_cs" it moght not have worked.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure with my above comment, it's just an idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we just compare data which is read from database, not from an SQL query, so I think it is not related.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, silly me. The comparison happens in PHP and not in a query in SQL. Seems I was blind.

{
return;
}
Expand Down