Jump to content

Colonel_mortis

Clients
  • Posts

    1,452
  • Joined

  • Last visited

  • Days Won

    5

 Content Type 

Downloads

Release Notes

IPS4 Guides

IPS4 Developer Documentation

Invision Community Blog

Development Blog

Deprecation Tracker

Providers Directory

Forums

Events

Store

Gallery

Posts posted by Colonel_mortis

  1. 2 hours ago, Marc Stridgen said:

    This is correct at the present time. If a post is already approved, and the user has the ability to edit, they would indeed be able to edit that without moderation

    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.

  2. 6 hours ago, Robert Angle said:

    If you use Member Groups to put members in content moderation, then you should also be able to edit that groups time limit for editing posts. For example, my moderated users as well as my unmoderated users have a 10 minute window to edit whatever they want. But when times up, their post is mine, lol 😄

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

  3. 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

  4. 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...?

  5. 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).

  6. 10 hours ago, Nathan Explosion said:

    Check for the optional patch in your ACP.

    That patch doesn't fix it

    10 hours ago, Colonel_mortis said:

    even with the latest 107667 build that deleted references to a couple more of the tasks

     

    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.

  7. 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!

  8. 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.

  9. 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.

  10. 1 hour ago, Stuart Silvester said:

    Interesting, that column was added about two years ago and we checked the table structures in 4.6.

    If you run the following, does it complete without error and add the missing column?

    \IPS\cms\Databases::checkandFixDatabaseSchema( 1 );

     

    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.

  11. 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}

     

  12. 22 hours ago, Daniel F said:

    I guess it's not exactly what you're suggesting, but couldn't you use the link to the edit form? modcp/alerts/?id=1&action=create
    That's what I do when I need to link to one.

     

    That said, I have raised an internal suggestion for this

    It is functional, but pretty ugly

  13. Firefox, Linux

    Could contain: Text, Computer Hardware, Hardware, Electronics

    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.

  14. To reproduce:

    • (Prerequisites: lazy loading is enabled (on by default), and the default disk storage location is used for attachments)
    • Attach an image to a post
    • Look at that post in the DB - it should look something like
      <img class="ipsImage ipsImage_thumbnailed" data-fileid="1" data-ratio="75.00" width="640" alt="foo.jpg" data-src="<fileStore.core_Attachment>/monthly_2022_10/foo.jpg" src="<___base_url___>/applications/core/interface/js/spacer.png" />
    • Edit the post (don't need to change anything)
      Look at the post again in the DB - it should now look something like
      <img alt="foo.jpg" class="ipsImage ipsImage_thumbnailed" data-fileid="1" data-ratio="75.00" width="640" data-src="<___base_url___>/uploads/monthly_2022_10/foo.jpg" src="<___base_url___>/applications/core/interface/js/spacer.png" />
    • Notice that the data-src has changed from <fileStore.core_Attachment>/monthly_2022_10/foo.jpg to <___base_url___>/uploads/monthly_2022_10/foo.jpg, so it is no longer agnostic of the backing file store

    Moving the attachments filestore will now cause these images to point to the wrong place.

    This is because in \IPS\Text\Parser::_parseImgElement, you have

    		/* Lazy-loaded? */
    		$srcAttribute = ( $element->hasAttribute( 'src' ) AND $element->hasAttribute( 'data-src' ) ) ? 'data-src' : 'src';
    
    		// ...
    
    		/* Or an attachment? */
    		elseif ( $attachment = static::_getAttachment( $element->getAttribute($srcAttribute ), $element->hasAttribute('data-fileid') ? $element->getAttribute('data-fileid') : NULL ) )
    		{
    			$file = $this->_getFile( 'core_Attachment', $element->getAttribute( $srcAttribute ) );
    
    			$element->setAttribute( 'data-fileid', $attachment['attach_id'] );
    			$element->setAttribute( 'src', str_replace( array( 'http:', 'https:' ), '', str_replace( static::$fileObjectClasses['core_Attachment']->baseUrl(), '{fileStore.core_Attachment}', $element->getAttribute( $srcAttribute ) ) ) );

    the last line there is setting the 'src' attribute, when in fact it should be setting $srcAttribute. The src attribute is subsequently discarded to replace with the lazy load placeholder.

    This works for new posts because $srcAttribute is 'src' there, but when editing posts (with lazy loading enabled) it is actually data-src.

  15. I have some gallery images that are failing to load with

    Quote

    [[Template gallery/front/view/imageFrame is throwing an error. This theme may be out of date. Run the support tool in the AdminCP to restore the default theme.]]

    even on the default theme.

    In the DB the image_notes column is serialized using PHP serialization rather than JSON:

     a:1:{i:0;a:6:{s:2:"id";s:32:"f8ea194da97843715f1a41ceae34e3a9";s:3:"top";i:343;s:4:"left";i:76;s:5:"width";i:52;s:6:"height";i:52;s:4:"note";s:42:"Crucial M55 and 8GB of Corsair Vengeance. ";}}

    which throws off the deserialization in \IPS\gallery\Image::get__notes and subsequently the \count( $image->_notes ) in imageFrame.phtml.

    This seems to be the case for all of the images with an image_date value prior to Jan 2016 (when we upgraded from IPB 3.x to IPS 4.x) that have a non-empty image_notes column.

    I am fixing them manually for my site, although it's definitely not a high priority issue because it has presumably been an issue for years.

  16. \IPS\nexus\Package, L2349, you have a comment saying

    Quote

    Set the cookie for 20 minutes - if the guest hasn't checked out by then they may no longer see their cart on every page but as long as their session is still valid they will still be able to check out.

    However, if this case does get hit, there is also a risk that this user's page gets cached, so the user's cart gets shown to other guests, which would be kinda bad. Also, theoretically they won't be able to check out because the cart page may also be cached as empty or incorrect, though that adds additional constraints so is slightly less likely.

    Given that this hasn't been hit throughout the whole life of guest page caching being an actual feature, it's probably not super critical - it's probably super rare that someone adds stuff to their cart, then browses the site for 20 mins+ without clearing it - but I don't see any downside to setting the cookie for the session lifetime, to be consistent with everywhere else, and this does matter when caching is pushed to the CDN.

  17. I have my own hooks to add additional spam filter rules, and I imagine I'm not alone. I want them to be able to take advantage of showing the reason that the post got mod queued, but there's no easy way to do that, because the post ID is only available after it's saved, and there's no suitable hook point in \IPS\Content::checkProfanityFilters between saving and constructing the approval entry.

    It would make me much happier if you replaced the switch statement on line 773 with either:

    • A method call that we can hook
    • Or fall back to setting $log->held_data = ["match" => $filtersMatched["match"]]
    • Or just always do that, and drop the logic to choose between the cases entirely

    You already made Approval itself hookable by pulling availableReasons out into a method (though why it's necessary to have it at all is a bit confusing to me), so this feels like an unfortunate oversight.

    I believe the image hold log in \IPS\Content::shouldTriggerProfanityFilters on line 639-643 is also going to hit the issue with the post ID not being available yet, but I obviously don't pay enough to be able to test that. The changes suggested here should make that code easier to unify with the rest of the cases.

  18. To reproduce:

    • Add a word to the banned words filter, such as "spam"
    • Make a new topic, including "spam" in the initial post
    • Observe that the topic got added to the approval queue as expected
    • Observe that there is no message saying that it was queued because it contains "spam"

    This is because for the first post in topics the approval entry is saved based on the topic, not the post itself, but there's no logic in a topic to render that information (and hiddenBlurb only loads the post itself).

×
×
  • Create New...