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

"Attempt to assign property 'cleanURL' on bool" fix #43444

Open
wants to merge 4 commits into
base: 4.4-dev
Choose a base branch
from

Conversation

AlexanderCkm
Copy link
Contributor

Pull Request for Issue #43443.

Summary of Changes

This change addresses an error which can occur if Smart Search results are viewed while it is still indexing. When indexing an item, the 'object' column is initially reset to an empty value as can be seen here. The 'object' column is then set again once indexing for that item is complete. If the user tries to view Smart Search results during indexing, and their results contain the item that the indexer is currently working on, they will encounter the "Attempt to assign property 'cleanURL' on bool" 500 error. This is because it is trying to deserialize the empty 'object' and then set the 'cleanURL' variable, but because it is an empty object, is it is deserialized to 'false'.

This is especially prominent with large finder indexes where there is larger window for the user to encounter a result with an empty 'object' value.

The proposed fix ensures the Search model won't return any results which are missing their 'object' value, and so in the instances where the user would normally get a 500 error, the problematic result will just be omitted instead until it has been indexed.

Testing Instructions

  1. Start the Smart Search indexer in the Joomla admin panel.
  2. While the indexer is running, access a Smart Search results page that includes the result with the manually emptied 'object' value.
  3. Observe that a 500 error occurs.

As the issue only exhibits for a small window during the indexing process, this can be tested reliably by run the Smart Search indexer and then manually set the 'object' column of one of the rows in the 'jos_finder_links' table to an empty value. Go to a Smart Search results page which includes the result with the empty 'object' value and you should see the 500 error. This essentially forces it into the state that it is seen temporarily during index.

Actual result BEFORE applying this Pull Request

Attempting to browse Smart Search results while indexing can result in the error: "Attempt to assign property 'cleanURL' on bool."

Expected result AFTER applying this Pull Request

Browsing Smart Search results while the finder is indexing should no longer have a chance to result in an error.

Link to documentations

Please select:

  • No documentation changes for docs.joomla.org needed
  • No documentation changes for manual.joomla.org needed

@richard67
Copy link
Member

richard67 commented May 8, 2024

@AlexanderCkm Are you sure that this will work with all supported versions of MySQL, MariaDB and PostgreSQL? I'm asking because on MySQL and MariaDB the object column is a mediumblob, so a binary data type, see https://github.com/joomla/joomla-cms/blob/4.4-dev/installation/sql/mysql/extensions.sql#L278 , and on PostgreSQL it is a bytea, also a binary data type, see https://github.com/joomla/joomla-cms/blob/4.4-dev/installation/sql/postgresql/extensions.sql#L266 . Depending on the database type and version there might be some implicit type casting so the check for an empty string works, but this might be different for other database types or versions.

So it might possibly be safer to check for $query->where('object IS NOT NULL');.

Furthermore you should use correct names quoting for the column as object might be a reserved word. So please use:

$query->where($db->quoteName('object') . ' IS NOT NULL');.

@AlexanderCkm
Copy link
Contributor Author

@AlexanderCkm Are you sure that this will work with all supported versions of MySQL, MariaDB and PostgreSQL? I'm asking because on MySQL and MariaDB the object column is a mediumblob, so a binary data type, see https://github.com/joomla/joomla-cms/blob/4.4-dev/installation/sql/mysql/extensions.sql#L278 , and on PostgreSQL it is a bytea, also a binary data type, see https://github.com/joomla/joomla-cms/blob/4.4-dev/installation/sql/postgresql/extensions.sql#L266 . Depending on the database type and version there might be some implicit type casting so the check for an empty string works, but this might be different for other database types or versions.

So it might possibly be safer to check for $query->where('object IS NOT NULL');.

Furthermore you should use correct names quoting for the column as object might be a reserved word. So please use:

$query->where($db->quoteName('object') . ' IS NOT NULL');.

Thanks for the quick response. I have just tried checking for not null instead of an empty string, but it looks like the empty string isn't being treated as null as I get the 500 error. My testing for this was done on MariaDB. Perhaps we can check for both not an empty string and not null for a solution that can work across the different types of databases? I have updated the PR to use quoteName and to additionally check for not null.

@AlexanderCkm
Copy link
Contributor Author

The AppVeyor build failed, but I believe this is unrelated to the code change. Please could it be ran again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants