getting issue while uploading file in serenity "an error occurred while scanning the uploaded file for viruses"

[MinalRaorane]

[LSNicholls]

Assuming you are sure that there is nothing really wrong with the file you are uploading... I am looking forward to hearing more information or other people's experience about this.

I found that I had to disable ClamAV on a production server because it was somehow competing with the installed virus scanning stuff on that server. The IT guys assured me that this would be okay to do, and I had no experience with the anti-virus stuff they were using.

[MinalRaorane]

@LSNicholls Thanks for help. I am using same pdf on local which is working properly. It is downloaded from secure source. I am getting above issue only while uploading on production server. I will ask IT person to check on production server for any such antivirus software setting.

[MinalRaorane]

AS I more check on production server windows event log I found following error:

Exception:
Serenity.Services.ValidationError: An error occured while scanning the uploaded file for viruses!
at Serenity.Extensions.ClamAVUploadScanner.Scan(Stream stream, String filename) in /_/src/Serenity.Extensions/Modules/ClamAV/ClamAVUploadScanner.cs:line 83
at Serenity.Web.DefaultUploadProcessor.Process(Stream stream, String filename, IUploadOptions options) in C:\Sandbox\startsharp\Serenity\src\Serenity.Net.Services\Upload\DefaultUploadProcessor.cs:line 41

[LSNicholls]

Hi -
Update: my previous concerns about the possible interaction between different anti-virus engines were completely misplaced. See additional messages in this discussion, below.

sorry I don't feel like I am helping, just chiming in. Again, looking forward to hearing anybody else's experience.

I am using same pdf on local which is working properly. It is downloaded from secure source.

This matches my experience, as does the Exception that you see in Production. Of course, to be fair, the anti-virus software isn't the only difference between local/dev environment and Production; OS would be different as well.

Also, I should point out, just in case, that I happen to be using the MultipleFileUploadEditor, with these options set:

 [DisplayName("Upload here")]
 [MultipleFileUploadEditor( CopyToHistory = false, IgnoreInvalidImage = true,
      FilenameFormat = "Projects/|ProjectFolder|/|Subfolder|/{4}", AllowNonImage = true)]

I would tell you what the anti-virus software was on the Production server, but I've honestly forgotten what they were using before. They have changed recently and I plan to re-test with their new software next time I do a test deployment, but it will be a while before I do that and report results here, and will let you know what the new anti-viral suite is as well, whether it works alongside ClamAV or not.

[VictorTomaili]

Changelog

Disable clamav or install clamav
https://github.com/serenity-is/serene/blob/master/src/Serene.Web/appsettings.json#L24

[LSNicholls]

@VictorTomaili I find your response somewhat mystifying, although I do thank you for causing me to look into the code further.

I understand this (now) based on the notes in ClamAVUploadScanner.cs, not from the ChangeLog or the appsettings file, both of which I had read, and thought I understood, previously.

Here's why the links, and your response, were mystifying to me: yes there is a note in the changelog for 6.7.0, which is aimed at people who are upgrading an existing project:

• Added the option to use ClamAV (https://www.clamav.net/) as an antivirus scanner for temporary uploads. To enable it, add services.ConfigureSection<Serenity.Extensions.ClamAVSettings>(Configuration); and services.AddSingleton<IUploadAVScanner, Serenity.Extensions.ClamAVUploadScanner>(); to your Startup.cs after upgrading to Serenity/Serenity.Extensions 6.7.0+. Consult ClamAV documentation on how to install it on your platform. This feature will be enabled by default once these changes have been made in Startup.cs. If you want to disable it for development purposes, set ClamAV:Enabled to false in your appsettings.Development.json (not recommended for production!).

However, if you are starting a project on 6.7.0+, the two change to Startup.cs are made by default and ClamAV is set enabled by default in appsettings.json as well. Meanwhile, in appsettings.Development.json, ClamAV is set disabled, also by default, which is why we would not notice anything was wrong until we deployed to Production.

You made use of ClamAV the default behavior without explicitly specifying a requirement for deployment (installing ClamAV) and then you masked the behavior in development environments. Do you really think this is appropriate? Do you really think your answer was sufficient?

There is no mention of this deployment or configuration requirement in Getting Started, Troubleshooting, FAQs, etc. Or a link in Tools & Libraries to ClamAV, FWIW.

[volkanceylan]

Yes, if we left it disabled in development, anyone who created a new project from Serene/StartSharp would have issues during development and would have to install ClamAV. If we did not enable it for production, it would be considered a security hole as reported by that company. Why would you want it to be enabled in a development machine. I would prefer users to read change log or search issues instead of having a security hole. And error message cant tell you the main issue as the virus scanner interface can be implemented in another way

[volkanceylan]

And where would that info be listed. You see it in exception log. This reminds me of the feature of not running migrations on arbitrary db. Even if we listed it the guide, docs and the login page, people still didnt read any, and we got hundreds of repeated issues, even screenshots of login page with the message that explains what they should do. Eventually we removed that. We will not remove this security check.

[LSNicholls]

@volkanceylan I am going to answer both of your responses here.

I would prefer users to read change log

I absolutely did read the change log! It explains what will happen "after upgrade" and it also says that "This feature will be enabled by default once these features are enabled."

The change log didn't explain what the status would be for projects using the new template! and you used passive voice ("will be enabled", "are enabled"), which didn't tell me WHO was going to be enabling them and WHEN.

All you had to do was add this information to the changelog regarding 6.7.0+:

• the suggested lines are included in the default 6.7.0+ Startup.cs and appsettings.json
• by default appsettings.development.json turns them off, so that if you are using all default settings you are not testing the new feature properly in your development environment

And where would that info be listed

See above for what I think should have been in the change log. As stated previously it wouldn't have been a terrible idea to put something in troubleshooting or FAQ or something, but that's a secondary quibble, not a primary concern.

Why would you want it to be enabled in a development machine.

I would think that would be obvious: All deployment procedures and features should be tested thoroughly before going to production. Consider that somebody might face issues in installing the ClamAV server in a production environment, which they should be able to practice and anticipate. They might need to be providing this information to a separate IT group responsible for the server, in fact.

You see it in exception log.

No, you see "An error occured while scanning the uploaded file for viruses!" Keep reading.

And error message cant tell you the main issue as the virus scanner interface can be implemented in another way

I do appreciate that your architecture makes it possible to use a different scanner. However, in ClamAVUploadScanner.cs, you recognized the problem and dealt with it in the outer catch, with a much better message ("Error occured during AV scan (is ClamAV installed?").

The problem is that we are falling into case ClamScanResults.Error, because ClamClient doesn't throw an error if it can't reach the server. So, maybe you can change the message in the case ClamScanResults.Error , right?

Here are my thoughts on how to do that:

The word "during" indicates that a scan actually occurred although unsuccessfully, which is not the case in the error we are facing. This usage is what confused me, and maybe others. Instead, how about, "Either an error occurred while AV processing took place or the AV server could not be reached." This would be something like your outer catch message, and it acknowledges that a scan may not have taken place at all.

instead of having a security hole.

Nothing I am suggesting provides a security hole. I am not complaining about the decisions you made in implementation, just about how you publicized/published them, and the way they are communicated, both in this discussion and in the error messages.

This reminds me of the feature of not running migrations on arbitrary db.

It is a good point to remember, and to remind me, that you have gone through this with multiple features. Again, I would never tell you to remove the feature, but I am trying to help you with the problems you face here, and are likely to face again with other features in the future as you make some hard decisions. It is always a very difficult call and it is your responsibility, which I recognize you work very hard to fulfill.