AWS Simple Email Service with Bounce Management

[AlexWebsites]

24 minutes ago, jesuralem said:

on my server this is clearly not a size issue on download/installation. 

But maybe it is too big for the scanner to be able to scan it though...

Same here and I can adjust my post_max_size but maybe the scanner just hangs because its overloaded. The scanner in ACP does not provide any output for me, just hangs.

[Jon Erickson]

I finally recreated the issue in my local environment and received the following stack trace. Confirmed it is indeed attributed to the compatibility scanner and trying to scan the entire vendor/dependency folder.

#	Time	Memory	Function	Location
1	0.0014	406056	{main}( )	.../index.php:0
2	0.1819	9685824	IPS\_Dispatcher->run( )	.../index.php:13
3	0.1820	9687328	IPS\core\modules\admin\support\_support->execute( )	.../Dispatcher.php:153
4	0.1820	9687328	IPS\Dispatcher\_Controller->execute( )	.../support.php:48
5	0.1820	9687328	IPS\core\modules\admin\support\_support->getBlock( )	.../Controller.php:107
6	0.1820	9687376	IPS\core\modules\admin\support\_support->_showBlockHookscanner( )	.../support.php:228
7	0.1845	9851896	IPS\Application\_Scanner::scanCustomizationIssues( $getDetails = FALSE, $shallowCheck = TRUE, $limit = 500, $options = ['shallowCheckEnabledOnly' => TRUE, 'enabledOnly' => FALSE] )	.../support.php:519
8	0.1983	9938336	IPS\Application\_Scanner::scanExtendedClasses( $getDetails = FALSE, $shallowCheck = TRUE, $limit = 500, $hardLimit = 5000, $options = ['shallowCheckEnabledOnly' => TRUE, 'enabledOnly' => FALSE] )	.../Scanner.php:786
9	21.4599	13207736	IPS\Application\_Scanner::getClassDetails( $scriptContents = '<?php\n// This file was auto-generated from sdk-root/src/data/apigateway/2015-07-09/api-2.json\nreturn [ \'version\' => \'2.0\', \'metadata\' => [ \'apiVersion\' => \'2015-07-09\', \'endpointPrefix\' => \'apigateway\', \'protocol\' => \'rest-json\', \'serviceFullName\' => \'Amazon API Gateway\', \'serviceId\' => \'API Gateway\', \'signatureVersion\' => \'v4\', \'uid\' => \'apigateway-2015-07-09\', ], \'operations\' => [ \'CreateApiKey\' => [ \'name\' => \'CreateApiKey\', \'http\' => [ \'method\' => \'POST\', \'requestUri\' => \'/apikeys\', \'responseCode\' => 20'... )	.../Scanner.php:175

Running into a max execution time. 

[AlexWebsites]

Thanks for troubleshooting this. Any idea on next steps? Can you call these files externally? 

[Jon Erickson]

2 hours ago, AlexWebsites said:

Thanks for troubleshooting this. Any idea on next steps? Can you call these files externally? 

Gonna have to work with IPS to come up with a solution. At this point, there is nothing we can do to avoid the issue unfortunately - besides removing the dependencies we are using, which are needed to talk with AWS. 

Edited October 11, 2022 by Jon Erickson

[jesuralem]

5 hours ago, Jon Erickson said:

Gonna have to work with IPS to come up with a solution.

Given how IPS seems to be open to discussion lately this is does not look like a good news...

[Daniel F]

5 hours ago, Jon Erickson said:

besides removing the dependencies we are using, which are needed to talk with AWS.

But you don't need the whole library, or?  Why would the SES stuff need Alexa, AI,CloudWatch, etc components?
One could literally remove all this stuff to decrease the size of the package which is exactly what few people have done with their packages. That's also what I'm doing in https://invisioncommunity.com/files/file/10135-seo-essentials/ where I included the google client API which was literally 50MB, so I had to remove everything unnecessary to get it under 10MB.

I get that it sucks and it's definitly not the best way, but given that where's not going to add official support for composer installations/update commands, it's the only way and what I was advising since people started shipping their apps with huge vendor folders.

[jesuralem]

@Jon Erickson looks like you are on you own on this one... can't say i'm surprised.

[Dll]

He's been given the solution above. 

[jesuralem]

i would say he's been given one solution where he is the only one having actual work to do.

He proposed another one where IPS may exclude some files form the PHP scanner...

[Dll]

And how would they know which to exclude? He's the app developer, it's on him to only include the files relevant to the app, not for invision to guess which those are and exclude them from a scanner. 

Edited October 12, 2022 by Dll

[Daniel F]

We’re indeed also looking at excluding the vendor directory, but it’s IMO a bad solution.

[Jon Erickson]

How is excluding the vendor directory a bad solution? Composer is the industry standard and not supporting it, when you want active developer engagement to help grow your ecosystem blows my mind. IPS should not be responsible for ensuring php compatibility in 3rd party dependencies. There’s a reason composer has versioning and constraints. To make sure libraries are compatible with specific php versions. I should not be responsible for modifying someone else’s work to make it work with IPS. That’s backwards. 

[Jon Erickson]

2 hours ago, Dll said:

And how would they know which to exclude? He's the app developer, it's on him to only include the files relevant to the app, not for invision to guess which those are and exclude them from a scanner. 

It’s extremely easy to exclude the vendor directory. It’s the standard folder name for composer. I’m surprised they are even scanning it to begin with. It makes no sense to scan it because everything inside it is a 3rd party dependency (aka not the developers own work) so if a problem were to arise, they would have to approach the dependency author for a solution. Shouldn’t be IPS problem to ensure 3rd party dependency compliance. 

[Stuart Silvester]

8 minutes ago, Jon Erickson said:

How is excluding the vendor directory a bad solution? Composer is the industry standard and not supporting it, when you want active developer engagement to help grow your ecosystem blows my mind. IPS should not be responsible for ensuring php compatibility in 3rd party dependencies. There’s a reason composer has versioning and constraints. To make sure libraries are compatible with specific php versions. I should not be responsible for modifying someone else’s work to make it work with IPS. That’s backwards. 

It's a bad idea from the view of what the code scanner does. Dependencies shipped via composer can still have bugs that would cause a site to be unusable.

[Daniel F]

That said, we're preparing a patch which should be available later today, where the vendor directory will be ignored! 

[Jon Erickson]

5 minutes ago, Stuart Silvester said:

It's a bad idea from the view of what the code scanner does. Dependencies shipped via composer can still have bugs that would cause a site to be unusable.

I completely agree. And I recognize this. But is it your responsibility to enforce this?

3 minutes ago, Daniel F said:

That said, we're preparing a patch which should be available later today, where the vendor directory will be ignored! 

Thank you for the cooperation. 

[Daniel F]

1 minute ago, Jon Erickson said:

But is it your responsibility to enforce this?

It's tricky, because once their system is broken, we have to jump in and fix it because it's unfortunately most of the time going to be literally completly broken and bricked with PHP>8 and not just some silenced warnings like before.

[jesuralem]

35 minutes ago, Stuart Silvester said:

It's a bad idea from the view of what the code scanner does. Dependencies shipped via composer can still have bugs that would cause a site to be unusable.

I have to say that for the moment it is scanning it that causes the sites to be unusable 🙂

At the very least the scanner should handle the situation better and manage a way to let the admins access the site.

[Jon Erickson]

1 minute ago, jesuralem said:

I have to say that for the moment it is scanning it that causes the sites to be unusable 🙂

At the very least the scanner should handle the situation better and manage a way to let the admins access the site.

Seems like making it a background task would be helpful. Does it really run with every script execution when on the support page?

[Stuart Silvester]

9 minutes ago, jesuralem said:

I have to say that for the moment it is scanning it that causes the sites to be unusable 🙂

At the very least the scanner should handle the situation better and manage a way to let the admins access the site.

We've released a patch with some changes, you're welcome to apply it from your AdminCP.

6 minutes ago, Jon Erickson said:

Seems like making it a background task would be helpful. Does it really run with every script execution when on the support page?

It becomes less important as we move the minimum requirement to PHP 8, if there's an issue with the code loading at that point areas of/the community won't work. The important part is scanning during the upgrade process and trying to detect issues that would cause a fatal error.

[Jon Erickson]

2 minutes ago, Stuart Silvester said:

We've released a patch with some changes, you're welcome to apply it from your AdminCP.

It becomes less important as we move the minimum requirement to PHP 8, if there's an issue with the code loading at that point areas of/the community won't work. The important part is scanning during the upgrade process and trying to detect issues that would cause a fatal error.

Thanks for the support. 

[beats23]

Glad to see this issue is now being rectified. 

[jesuralem]

yes it is working on my test site.

Now PHP8 does not work but this is another matter.

[AlexWebsites]

12 hours ago, Stuart Silvester said:

We've released a patch with some changes, you're welcome to apply it from your AdminCP.

I just updated to 4.7.3 because I did not see a patch. Is it included in the distribution? I still don't see it. Does it take time to show up?

 

When I run the compatibility checker, I still have the same issue so I'm assuming the patch isn't applied yet. Can someone share the patched file or code change?

[Jon Erickson]

Looks like it was shipped with 4.7.3. Line 78 of Scanner.php:

$excludeDirs = [ 'hooks', 'setup', 'dev', 'vendor' ];

HOWEVER, the patch, as it stands right now, will not work as the regex pattern is looking for a directory path of

/\/var\/www\/html\/applications\/awsses\/(?:hooks|setup|dev|vendor)/

The vendor folder is located in the sources directory as required by IPS, not the root application folder.

@Daniel F, do I move my vendor folder to the root directory or do you guys update the regex to look for the vendor folder in the sources directory? 

For the time being, you can edit line 78 of Scanner.php to the following to fix the issue.

$excludeDirs = [ 'hooks', 'setup', 'dev', 'vendor', 'sources\/vendor' ];

Edited October 13, 2022 by Jon Erickson