Colonel_mortis

The bug still exists on 4.7.20 (107802) (I have no visibility into v5, but I'm guessing this code hasn't changed) Error: Object of class IPS\Member could not be converted to string (0) #0 /opt/forum/system/Db/Select.php(407): IPS\_Db->preparedQuery() #1 /opt/forum/system/Db/Select.php(465): IPS\Db\_Select->runQuery() #2 /opt/forum/system/Db/Select.php(389): IPS\Db\_Select->rewind() #3 /opt/forum/system/Content/Api/CommentController.php(505): IPS\Db\_Select->first() #4 /opt/forum/applications/forums/api/posts.php(346): IPS\Content\Api\_CommentController->_report() #5 /opt/forum/system/Api/Controller.php(180): IPS\forums\api\_posts->POSTitem_report() #6 /opt/forum/system/Dispatcher/Api.php(345): IPS\Api\_Controller->execute() #7 /opt/forum/api/index.php(11): IPS\Dispatcher\_Api->run() #8 {main}Fix should be diff --git a/system/Content/Api/CommentController.php b/system/Content/Api/CommentController.php index 3e68f7acf..a37aef50a 100644 --- a/system/Content/Api/CommentController.php +++ b/system/Content/Api/CommentController.php @@ -502,7 +502,7 @@ public function _report( $id ): \IPS\Api\Response try { $index = \IPS\core\Reports\Report::loadByClassAndId( \get_class( $object ), $object->$idColumn ); - $report = \IPS\Db::i()->select( '*', 'core_rc_reports', array( 'rid=? and report_by=? and date_reported > ?', $index->id, $member, time() - ( \IPS\Settings::i()->automoderation_report_again_mins * 60 ) ) )->first(); + $report = \IPS\Db::i()->select( '*', 'core_rc_reports', array( 'rid=? and report_by=? and date_reported > ?', $index->id, $member->member_id, time() - ( \IPS\Settings::i()->automoderation_report_again_mins * 60 ) ) )->first(); /* They have aleady reported, so do nothing */ throw new \IPS\Api\Exception( 'REPORTED_ALREADY', '1S425/C', 404 );

Yes, I verified that this bug exists in 107801

\IPS\Content\Search\MySql::resetLastComment you do $latest = \IPS\Db::i()->select( 'index_object_id, index_date_updated, index_class', 'core_search_index', array( \IPS\Db::i()->in( 'index_class', $classes ) . ' AND index_item_id=? and index_hidden=0', $indexItemId ), 'index_date_created DESC', array( 0, 1 ) )->first();Followed later by \IPS\Db::i()->update( 'core_search_index', array( 'index_date_updated' => $latest['index_date_updated'], 'index_date_commented' => $latest['index_date_commented'] ), array( \IPS\Db::i()->in( 'index_class', $classes ) . ' AND index_item_id=? AND index_object_id=?', $indexItemId, ( $firstCommentId ?? $indexItemId ) ) );but index_date_commented was not loaded in the query, so this line fails IN_DEV with an array index not found exception.

In places where there are multiple content items displayed together, such as an activity feed, some items' images get grouped together in the lightbox, while others are are isolated. This is because in ips.core.lightboxedImages.js::addOrUpdateWrappingLink, `self` has not been defined within } else if( !closestLink.length ) { image.wrap( $( '<a href="' + imageSrc + '"' + " title='" + ips.getString('enlargeImage') + "' data-fileId='" + fileId + "'data-wrappedLink data-ipsLightbox data-ipsLightbox-group='" + self._random + "'></a>" ) ); } and thus falls back to globalThis.self, which doesn't have an _random attribute, so the attribute gets set to undefined. I'm not sure why some uploaded images are being wrapped in a link and others are not - that also feels a bit weird.

In firefox, pressing enter twice does not break out of quotes. This did used to work, and still does on Chrome, although I'm not sure why it did given the code involved was last changed 4 and 10 years ago in ipsquote and ckeditor respectively. It seems to be because the regex that removes the bogus <br/> does not account for the type="_moz" attribute that is added when running in firefox. Here's a fix that seems to work for me: diff --git a/applications/core/dev/ckeditor/plugins/ipsquote/plugin.js b/applications/core/dev/ckeditor/plugins/ipsquote/plugin.js index b26116dd..9404a002 100644 --- a/applications/core/dev/ckeditor/plugins/ipsquote/plugin.js +++ b/applications/core/dev/ckeditor/plugins/ipsquote/plugin.js @@ -166,7 +166,7 @@ CKEDITOR.plugins.add( 'ipsquote', { var paragraphContent = selectedElement.getHtml(); // We are going to ignore <br> tags, but only one (if there are more than one don't ignore) - paragraphContent = paragraphContent.replace( /\<br\>/, '' ); + paragraphContent = paragraphContent.replace( /\<br( [^>]*)?\>/, '' ); // Ignore empty span tags paragraphContent = paragraphContent.replace( /\<span.*?\>\s*?\<\/span\>/g, '' ); Originally reported to me by a user.

I will have a look at that, although it would still be great if you could make this drop-in change to fix it for everyone.

In China, google.com is blocked, which also affects recaptcha. Google's docs recommend using recaptcha.net rather than google.com to work around that - it should just be a drop-in replacement for the places where you're using google.com for recaptchas. This was flagged to me by a user.

That's not true today for replies (they get requeued for approval), only new topics. It also defeats the point of content moderation if they can post something ok and later make it bad.

I shouldn't need to do that though, it works for posts within a topic.

If a user has content moderation enabled, any new topics or posts need to be approved by a moderator, and any edits to existing non-topic-startong posts. However, editing the topic's original post does not need to be approved. Especially when used to combat spam, this is a pretty big loophole. To reproduce: * Enable content moderation for a user * Have that user post a new topic - moderator approval will be required * Have a moderator approve the topic * Have the user go back and edit that post - moderator approval will incorrectly not be required

https://invisioncommunity.com/forums/topic/475949-uncaught-exception-when-page-query-param-is-invalid/?page=5%0a causes an uncaught exception because it tries to construct it with path /page/5%0a, which is invalid. You already guard other usages of the param with intval, so you probably want that here too.

This change, made in 4.7.12, breaks applications that don't have an extensions.json or that haven't got all their extensions registered there. Any app that created extensions a while ago (and hasn't updated them recently?) will silently have stopped working. diff --git a/system/Application/Application.php b/system/Application/Application.php index 66c5fc8cd..854837fa7 100644 --- a/system/Application/Application.php +++ b/system/Application/Application.php @@ -786,7 +786,6 @@ public function extensions( $app, $extension, $construct=TRUE, $checkAccess=FALS $classes = array(); $jsonFile = $this->getApplicationPath() . "/data/extensions.json"; - $directory = $this->getApplicationPath() . "/extensions/{$app}/{$extension}"; /* New extensions.json based approach */ if ( file_exists( $jsonFile ) and $json = @json_decode( \file_get_contents( $jsonFile ), TRUE ) ) @@ -814,56 +813,6 @@ public function extensions( $app, $extension, $construct=TRUE, $checkAccess=FALS } } } - - /* Legacy DirectoryIterator approach */ - elseif ( is_dir( $directory ) ) - { - $dir = new \DirectoryIterator( $directory ); - - foreach ( $dir as $file ) - { - /* Macs create copies of files with "._" prefix which breaks when we just load up all files in a dir, ignore those */ - if ( !$file->isDir() and !$file->isDot() and mb_substr( $file, -4 ) === '.php' AND mb_substr( $file, 0, 2 ) != '._' ) - { - $classname = 'IPS\\' . $this->directory . '\extensions\\' . $app . '\\' . $extension . '\\' . mb_substr( $file, 0, -4 ); - - /* Check if class exists - sometimes we have to use blank files to wipe out old extensions */ - try - { - if( !class_exists( $classname ) ) - { - continue; - } - - if ( method_exists( $classname, 'deprecated' ) ) - { - continue; - } - } - catch( \ErrorException $e ) - { - continue; - } - - if ( method_exists( $classname, 'generate' ) ) - { - $classes = array_merge( $classes, $classname::generate() ); - } - elseif ( !$construct ) - { - $classes[ mb_substr( $file, 0, -4 ) ] = $classname; - } - else - { - try - { - $classes[ mb_substr( $file, 0, -4 ) ] = new $classname( $checkAccess === TRUE ? \IPS\Member::loggedIn() : ( $checkAccess === FALSE ? NULL : $checkAccess ) ); - } - catch( \RuntimeException $e ){} - } - } - } - } return $classes; } Aren't we past this yet...?

FastCGI sent in stderr: "PHP message: PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 10485760 bytes) in [...]/system/Node/Statistics.php on line 118" while reading response header from upstream, client: [], server: [], request: "GET /forum/17-[...]/?sortby=views&sortdirection=desc HTTP/2.0" That's in authorsPostedIn. It looks like there's two places where that gets called - computing the content that the current user has posted in, and computing which postedIn indicator to show. The query that gets run for this will load about 260k entries (there are a bunch of topics with a very large range of contributors). I'm surprised that's enough to OOM PHP with 128M memory, but it seems to be the case. For computing content that the current user has posted in, you can do a way more efficient query by filtering by user. For computing the postedIn indicator, you could just skip it entirely on installs where that feature isn't used, but also I think you could make it much more efficient by joining the members in the query to just end up with the group IDs (though that query is probably a bit non-trivial).

If the account is validating or partially registered (due to social logins), the cookie page isn't on the allowlist, so users get redirected back to the validating page when they try to view it.

It looks like someone figured it out today, the 107668 diff appears to fix this now.

That patch doesn't fix it I do have a custom upgrade script, so it's possible that I messed up and missed running some code that deletes the tasks, but I also reproduced it on a test install using the normal upgrader and can't find mention of any code that does perform the deletes.

The terminateHosting, monitor and expectedOutputMonitoring tasks were deleted in 4.7.9, but you don't clean up the tasks from the DB, so they get stuck and trigger the "tasks not running" warning (even with the latest 107667 build that deleted references to a couple more of the tasks). I'm pretty sure 3rd party devs have complained about this a few times before - deleting a task ought to automatically clean it up from the tasks table!

The new(ish) Images profile tab shows all the time, even if it has nothing to show. That's different to the other extensions (eg gallery albums, which is separate for reasons that elude me, or custom fields), which only show up if there's content to display. My personal opinion is that this tab is pretty pointless for most sites, because the same stuff is already available in the user's content page. I'm not bothered whether it stays or goes though, as long as it can stay out of the way when it's empty and just a waste of space.

There's no check that the user has permission to see the post being edited, so it's possible to edit a post after it has already been hidden. To reproduce: Post in a topic Start editing that post Moderator hides the post Submit the edit Edit goes through successfully It's not so bad in that repro, because they must have had it open already, but it's not great if the user constructs the calls manually to both load the current version of the post, and make changes to it.

Yeah that did it. There was a period where due to a bug on my side we weren't running some manual upgrade code (just the json-defined changes) - if it was meant to be changed in an upgrade, that's probably the root cause, in which case that's on me.

The background task to delete guest content from pages is spewing uncaught exceptions SELECT * FROM `cms_custom_database_1` WHERE record_author_name='<redacted>' ORDER BY primary_id_field LIMIT 0,250 Unknown column 'record_author_name' in 'where clause' #0 /opt/forum/system/Db/Select.php(388): IPS\_Db->preparedQuery() #1 /opt/forum/system/Db/Select.php(446): IPS\Db\_Select->runQuery() #2 [internal function]: IPS\Db\_Select->rewind() #3 /opt/forum/applications/core/extensions/core/Queue/MemberContent.php(93): IteratorIterator->rewind() #4 /opt/forum/system/Task/Task.php(47): IPS\core\extensions\core\Queue\_MemberContent->run() #5 /opt/forum/applications/core/tasks/queue.php(43): IPS\_Task::runQueue() #6 /opt/forum/system/Task/Task.php(375): IPS\core\tasks\_queue->IPS\core\tasks\{closure}() #7 /opt/forum/applications/core/tasks/queue.php(38): IPS\_Task->runUntilTimeout() #8 /opt/forum/system/Task/Task.php(274): IPS\core\tasks\_queue->execute() #9 /opt/forum/system/Task/Task.php(237): IPS\_Task->run() #10 /opt/forum/applications/core/interface/task/task.php(58): IPS\_Task->runAndLog() #11 {main}

It is functional, but pretty ugly

For moderator bookkeeping and discussion it would be helpful if you could direct link to a specific alert within modcp (to a page containing just that alert). Right now the only way to view an alert as a moderator is to scroll through the list of all of them, which is not useful when referencing it elsewhere.

http://localhost/ips/admin/?app=nexus&module=subscriptions&controller=subscriptions&do=convertToSubscription&id=1&csrfKey=3ed05127f5b65a77dcf8987de5da8e5c causes a whoops error because the CSRF key is left in (and is required).

Firefox, Linux When this notification was sent, there was only one new unread post in the topic, and I had no prior unread notifications (as judged by in-platform notifications). I'm a bit confused about where 6 has come from - it is less than the number of other members' replies to this topic (which I have been following since creation), but more than the number of replies since I last rebooted this computer. I usually don't clear my system notifications (the system notification list that these push notifications are delivered to), but I had manually cleared them prior to receiving this to see if that makes any difference. It didn't (or at least it didn't solve it, perhaps it would otherwise have come up with a different number?). I'm pretty sure this is something to do with the self.registration.getNotifications and unseenCount stuff in serviceWorker.js, but I'm not quite sure the semantics here (I assumed that clearing the notification list, or restarting my computer, would close all the open notifications). The correct behaviour here, in my mind, is that the numbers here should be no greater than the in-platform notification list - if there have been 3 notifications about replies to a given topic since I last read my in-platform notifications, the number that is shown here should represent the number of notifications about replies that are unread in both the in-platform notifications and push notifications (at most 3, but could be lower if I dismissed some native notifications). There are definitely alternative approaches that would make sense too, but it's pretty clear to me that the current approach is not correct. Maybe this is just a Firefox+Linux issue, I haven't tried reproing on any other platforms yet.