Duplicate Potential add-on incompatibility in Copy Post functionality

XCentral

XCentral

Well-known member
Hello,

We got a report from one of our customers regarding an SQL error when copying a post. Troubleshooting the issue we found that it is caused because of XenForo_Model_Post::_copyPost method doing database insert call using the $post array it gets as an argument. However, this array is gathered by XenForo_Model_Post::getPostsAndParentData method which internally calls XenForo_Model_Post::getPostsByIds. This last method is meant to be extended by third-party add-ons to join new tables and get new information from their own tables by overriding XenForo_Model_Post:: preparePostJoinOptions used by this method.

So our add-on joins its own table and adds some variables to the returned post array, causing the error reported.

After reproducing the issue we saw it causes problems with some other add-ons as well, e.g. XFA - Better Blogs.

We have implemented a temporary fix by overriding XenForo_Model_Post::getPostsAndParentData method (which seems to be used for inline moderation actions only so far) and removing the keys from the posts which are not found in xf_post table, using $db->describeTable call.

Please let us know your thoughts or if you need any further information.

Thank you!
 
Last edited:
Mike said:
See most post here: http://xenforo.com/community/thread...er-fields-as-present-in-the-datawriter.67851/

You're doing something which is very unexpected in the grand scheme of things (getting a post by ID with no other parameters returning data that isn't part of the table).
Click to expand...

Thank you for the reply. Adding some condition to preparePostJoinOptions that will make it fetch data only when thread is displayed will solve the problem, just like checking and unsetting non-existing keys solves it. The thing is that this can very easily break the functionality of any add-on doing something like this and introduce a bug which can take long time to find-out, as from the code itself we can't see any reason why adding additional field to the post would be considered something "very unexpected" - if I need some data to be added to the post array whenever the post array is available, I have to join the table on all cases. Another solution for that would be altering post table and adding fields to, but we have been using the join approach for long time because it has one advantage - we avoid altering post table as on a huge table this can potentially corrupt it, and adds large amount of data because of default values stored for all posts (which in reality never will have that data changed).

So, considering again how extendable XenForo is, the possibility to override all non-final methods of all models, the usage of post array as is for the insert query without any check may need to be reconsidered. Maybe creation of a datawriter and setting fields to it in a try-catch block can be used - the fields not set in the datawriter will throw an exception and they will be removed from the array in the catch block, and only after this the array will be inserted directly.

Please let us know your thoughts.

Thank you!
 
Back
Top Bottom