Re: [Clamav-devel] PATCH: Enhanced exclude / include support for clamscan
On 21 May 2009, at 3:19 pm, Tomasz Kojm wrote: > On Tue, 19 May 2009 15:49:23 +0100 > Mark Allan wrote: > >> It would also eliminate another issue I've had to work >> around which is the lack of case-insensitive regex matching - I could > > BTW, the SVN version of ClamAV supports case-insensitive regex > matching.You > just need to precede the regex with (?i), eg. > > clamscan --exclude-dir="(?i)^/SyS$" > > will skip /sys, /Sys, /sYs, /syS, /SYs, /SyS, /sYS, and /SYS :-) Yeah I spotted that this morning. Superb news on that and the --file- list option. Thanks :-) ___ http://lurker.clamav.net/list/clamav-devel.html Please submit your patches to our Bugzilla: http://bugs.clamav.net
Re: [Clamav-devel] PATCH: Enhanced exclude / include support for clamscan
On Tue, 19 May 2009 15:49:23 +0100 Mark Allan wrote: > It would also eliminate another issue I've had to work > around which is the lack of case-insensitive regex matching - I could BTW, the SVN version of ClamAV supports case-insensitive regex matching.You just need to precede the regex with (?i), eg. clamscan --exclude-dir="(?i)^/SyS$" will skip /sys, /Sys, /sYs, /syS, /SYs, /SyS, /sYS, and /SYS :-) -- oo. Tomasz Kojm (\/)\. http://www.ClamAV.net/gpg/tkojm.gpg \..._ 0DCA5A08407D5288279DB43454822DC8985A444B //\ /\ Thu May 21 16:18:36 CEST 2009 ___ http://lurker.clamav.net/list/clamav-devel.html Please submit your patches to our Bugzilla: http://bugs.clamav.net
Re: [Clamav-devel] PATCH: Enhanced exclude / include support for clamscan
On Tue, 19 May 2009 15:49:23 +0100 Mark Allan wrote: > > On the one hand, you're right that were clamscan to have a command- > > line option to tell it to read a list of files on stdin, the logic I > > built into it could be implemented externally, either with home- > > grown scripts or with a separate utility bundled with clamav. I > > certainly wouldn't be adverse to achieving my goal that way, where > > that functionality to be provided by clamscan. > > I think this way would be much better than changing the interface, and > would have the added benefit of being significantly more powerful than > you probably realise. If we could supply a list of files either on > stdin or within a single text file passed on the command line with eg. > a -f flag, users and admins could incorporate ClamAV into scripts/ > tasks and automated jobs a lot more easily than is currently > possible. It would also eliminate another issue I've had to work > around which is the lack of case-insensitive regex matching - I could > perform my own matching and just supply filenames within a temporary > file or via stdin. I added support for --file-list/-f in both clamscan and clamdscan in SVN. Thanks, -- oo. Tomasz Kojm (\/)\. http://www.ClamAV.net/gpg/tkojm.gpg \..._ 0DCA5A08407D5288279DB43454822DC8985A444B //\ /\ Thu May 21 16:07:38 CEST 2009 ___ http://lurker.clamav.net/list/clamav-devel.html Please submit your patches to our Bugzilla: http://bugs.clamav.net
Re: [Clamav-devel] PATCH: Enhanced exclude / include support for clamscan
>> The regular expressions being excluded or included are compiled >> just once and stored in memory Brilliant! > On the one hand, you're right that were clamscan to have a command- > line option to tell it to read a list of files on stdin, the logic I > built into it could be implemented externally, either with home- > grown scripts or with a separate utility bundled with clamav. I > certainly wouldn't be adverse to achieving my goal that way, where > that functionality to be provided by clamscan. I think this way would be much better than changing the interface, and would have the added benefit of being significantly more powerful than you probably realise. If we could supply a list of files either on stdin or within a single text file passed on the command line with eg. a -f flag, users and admins could incorporate ClamAV into scripts/ tasks and automated jobs a lot more easily than is currently possible. It would also eliminate another issue I've had to work around which is the lack of case-insensitive regex matching - I could perform my own matching and just supply filenames within a temporary file or via stdin. > Other applications in the same genre with similar functionality > include rsync, rdiff-backup, and GNU tar. All of those, like > clamscan, could force the user to implement exclude / include > functionality externally and then feed a file list into the > application, and yet all of them provide complex, flexible built-in > filtering functionality of their own. Indeed. I don't think anyone's suggesting that the existing functionality be removed. > Perhaps what is needed is a compromise -- basic filtering of the > sort I implemented, *plus* the "-f" flag (or the equivalent) for > people who want to roll their own logic? In my opinion, that would be an ideal solution. > Interfaces change. Such is life. Remaining wedded to broken > interfaces leads over time to difficult to maintain code with > inferior functionality and performance. The changes should be > visibly documented, and people who are using ClamAV should be smart > enough to read the release notes when they upgrade. The problem isn't with changing the interface, the problem is that your solution doesn't actually change the interface, it changes the implementation - you've just placed extra importance on the order of the items, which wasn't there before. This could have the subtle effect of still appearing to work, but no longer doing what the user expected. Files which should be scanned may end up getting missed and the user might not realise. If the interface actually changes, then that's not such a big deal as it will simply and very obviously break any existing procedures a user has set up, and they'll have no choice but to change how they work. Mark ___ http://lurker.clamav.net/list/clamav-devel.html Please submit your patches to our Bugzilla: http://bugs.clamav.net
Re: [Clamav-devel] PATCH: Enhanced exclude / include support for clamscan
Kamens, Jonathan wrote: > On the other hand, I don't quite view clamscan as being a pipeline > tool of the same genre as sed, tr, etc. I think of it as a > full-fledged application, and as such, I think it's reasonable for > this kind of functionality to be built into it if it's something that > a lot of people are going to find useful. > > Other applications in the same genre with similar functionality > include rsync, rdiff-backup, and GNU tar. All of those, like > clamscan, could force the user to implement exclude / include > functionality externally and then feed a file list into the > application, and yet all of them provide complex, flexible built-in > filtering functionality of their own. They also typically provide a '--with-files-from=' option which might be the right thing for clamscan too. /Per Jessen, Zürich ___ http://lurker.clamav.net/list/clamav-devel.html Please submit your patches to our Bugzilla: http://bugs.clamav.net
Re: [Clamav-devel] PATCH: Enhanced exclude / include support for clamscan
Thanks for the feedback. Please forgive the formatting of this response; I am unfortunately stuck using Outlook :-/, and I'm sure it's going to stick line breaks in the wrong places. >While these features might make a lot of sense, I can't help thinking >that this isn't the 'Unix' (right:) way to go about implementing them. I've been using Unix for over two decades and have been known to edit raw sendmail CF files, so I think I know a thing or two about the "Unix way" :-). On the one hand, you're right that were clamscan to have a command-line option to tell it to read a list of files on stdin, the logic I built into it could be implemented externally, either with home-grown scripts or with a separate utility bundled with clamav. I certainly wouldn't be adverse to achieving my goal that way, where that functionality to be provided by clamscan. On the other hand, I don't quite view clamscan as being a pipeline tool of the same genre as sed, tr, etc. I think of it as a full-fledged application, and as such, I think it's reasonable for this kind of functionality to be built into it if it's something that a lot of people are going to find useful. Other applications in the same genre with similar functionality include rsync, rdiff-backup, and GNU tar. All of those, like clamscan, could force the user to implement exclude / include functionality externally and then feed a file list into the application, and yet all of them provide complex, flexible built-in filtering functionality of their own. Perhaps what is needed is a compromise -- basic filtering of the sort I implemented, *plus* the "-f" flag (or the equivalent) for people who want to roll their own logic? It is also worth noting that I was not adding new functionality to the application but rather enhancing existing functionality. If there had been no --exclude and --include flags in clamscan to begin with, I probably would have implemented something like what you propose. But given that they were there and buggy (recompiling all the regexps for every file / directory being considered), I felt that fixing / enhancing the existing functionality was the right way to go. >You mentioned maintenance. IMO, what you're doing is asking for a >maintenance headache. You mentioned changing the API. Please, for >the sake of people that are using the package, already, don't do that >unless there is (a) a VERY compelling reason and (b) NO other way. Interfaces change. Such is life. Remaining wedded to broken interfaces leads over time to difficult to maintain code with inferior functionality and performance. The changes should be visibly documented, and people who are using ClamAV should be smart enough to read the release notes when they upgrade. If GNU tar, a far more widely used utility than ClamAV, can get away with changing its interface as frequently as it does, then I can't get too worked up over the relatively minor change I've proposed to the ClamAV interface. Having said that, if the maintainers think that this particular interface change is a bigger deal than I think it is, then there are alternatives, e.g., adding a flag to enable the new exclude / include behavior and emulating the old behavior if that flag isn't specified. Or changing the names of the options for the new behavior (although I cringe a bit at the thought of that, since "--exclude" and "--include" are the most obvious and intuitive names for the options). >I'm already leaning towards abandoning ClamAV because of instability >in the API. Sanesecurity is the only reason I continue to use it. It would seem, then, that the maintainers of ClamAV may share my philosophy about interface changes :-). Jik ___ http://lurker.clamav.net/list/clamav-devel.html Please submit your patches to our Bugzilla: http://bugs.clamav.net
Re: [Clamav-devel] PATCH: Enhanced exclude / include support for clamscan
Hi there, On Wed, 13 May 2009 Kamens, Jonathan > ... wanted to make clamscan a bit smarter ... > > * don't scan files more than 30 days old ... > * scan a rotating subset of those old files ... > * scan any files that were previously found to be infected ... > * don't scan files which have been explicitly suppressed > > ... I decided to go ahead and enhance clamscan ... > > I would be delighted to hear any feedback ... While these features might make a lot of sense, I can't help thinking that this isn't the 'Unix' (right:) way to go about implementing them. Would it not be better just to tell clamscan to accept the names of the files to scan on stdin (something like 'clamscan -f -' maybe?) and create a tool which can provide this list if one doesn't already exist? For most users, things like the standard 'find' or 'xargs' will be more than sufficient. You mentioned maintenance. IMO, what you're doing is asking for a maintenance headache. You mentioned changing the API. Please, for the sake of people that are using the package, already, don't do that unless there is (a) a VERY compelling reason and (b) NO other way. I'm already leaning towards abandoning ClamAV because of instability in the API. Sanesecurity is the only reason I continue to use it. -- 73, Ged. ___ http://lurker.clamav.net/list/clamav-devel.html Please submit your patches to our Bugzilla: http://bugs.clamav.net
Re: [Clamav-devel] PATCH: Enhanced exclude / include support for clamscan
On Tue May 12 2009 20:37:19 GMT+0200 (CEST)
Kamens, Jonathan wrote:
> I would be delighted to hear any feedback that anyone has about this
> patch. I would be even more delighted if the maintainers of ClamAV
> decided to fold my patch into an upcoming ClamAV release, so I don't
> have to maintain it myself forever :-).
Hi Jonathan,
please open a bug report ("enhancement") at bugs.clamav.net and attach
your patches there together with all the additional information. This
will make things easier for us, also you/users will have a better way to
control what's happening with the stuff.
Thanks,
--
oo. Tomasz Kojm
(\/)\. http://www.ClamAV.net/gpg/tkojm.gpg
\..._ 0DCA5A08407D5288279DB43454822DC8985A444B
//\ /\ Wed May 13 09:51:25 CEST 2009
___
http://lurker.clamav.net/list/clamav-devel.html
Please submit your patches to our Bugzilla: http://bugs.clamav.net
