No Code Attached Yet
avatar felixkat
felixkat
27 Jan 2025

Problem identified

In 2019 a "band aid" was put in by @alikon to prevent JSON data getting truncating when saving a module where the params data exceeded the 65535 limit.

#24635

I believe there have been multiple discussions in regards to increasing the field size in MySQL but the consensus is to keep it as it is for performance reasons.

We have a scenario where the data in "params" for a module exceeds the 65535 limit. We don't wish to save the data in the table but to save it as a file. To do this we run a routine within onExtensionBeforeSave However that routine will never run as the "check the data" routine happens before the "Trigger the before save event".

// Prepare the row for saving
        $this->prepareTable($table);

        // Check the data.
        if (!$table->check()) {
            $this->setError($table->getError());

            return false;
        }

        // Trigger the before save event.
        $result = Factory::getApplication()->triggerEvent($this->event_before_save, [$context, &$table, $isNew]);

        if (\in_array(false, $result, true)) {
            $this->setError($table->getError());

            return false;
        }

        // Store the data.
        if (!$table->store()) {
            $this->setError($table->getError());

            return false;
        }

Proposed solution

Logically I would have thought it should be Prepare, Trigger before save, Check the Data, Store. So I am proposing that the check and trigger change order.

// Prepare the row for saving
        $this->prepareTable($table);

        // Trigger the before save event.
        $result = Factory::getApplication()->triggerEvent($this->event_before_save, [$context, &$table, $isNew]);

        if (\in_array(false, $result, true)) {
            $this->setError($table->getError());

            return false;
        }

// Check the data.
        if (!$table->check()) {
            $this->setError($table->getError());

            return false;
        }


        // Store the data.
        if (!$table->store()) {
            $this->setError($table->getError());

            return false;
        }

Open questions

The above seems logically to me but I'm hoping a discussion would highlight any potential issues with this.

avatar felixkat felixkat - open - 27 Jan 2025
avatar joomla-cms-bot joomla-cms-bot - change - 27 Jan 2025
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 27 Jan 2025
avatar felixkat felixkat - change - 27 Jan 2025
The description was changed
avatar felixkat felixkat - edited - 27 Jan 2025
avatar alikon
alikon - comment - 5 Feb 2025

@felixkat can you submit a pull request with your proposal ?

avatar felixkat
felixkat - comment - 5 Feb 2025

@alikon Yes I can do this.

I have noticed that this sequence of events happens in other places throughout the Joomla Models.

Should I create a pull request for all of them to keep it consistent?

The change would run the "Trigger the before save event" before "checking the data". This allows 3rd parties to sanitise their data before Joomla performs the check before storing.

avatar alikon
alikon - comment - 6 Feb 2025

1st thank you for your help

i would say yes sounds "logical" to me 👍

avatar alikon alikon - change - 7 Feb 2025
Status New Closed
Closed_Date 0000-00-00 00:00:00 2025-02-07 08:32:18
Closed_By alikon
avatar alikon alikon - close - 7 Feb 2025
avatar alikon
alikon - comment - 7 Feb 2025

closing as we have a pull request #44834

Add a Comment

Login with GitHub to post a comment