search

kylefarris / clamscan

A robust ClamAV virus scanning library supporting scanning files, directories, and streams with local sockets, local/remote TCP, and local clamscan/clamdscan binaries (with failover).
MIT License
236 stars 69 forks source link

using scan_files without file_cb #3

Closed nicolaspeixoto closed 9 years ago

nicolaspeixoto commented 9 years ago

Hello, I found a bug when i'm trying to use scan_files without file_cb parameter:

clam.scan_files(files, function(err, good_files, bad_files) {});

This was printed to console:
if(this.settings.debug_mode === true) ^ TypeError: Cannot read property 'debug_mode' of undefined at home/user/dev/node_modules/clamscan/index.js:262:22

After i had correted this, it was printed: return end_cb(err, path, null); ^ ReferenceError: path is not defined at home/user/dev/node_modules/clamscan/index.js:264:25

kylefarris commented 9 years ago

Thanks for the bug report Nicolas! I see you've put in a PR. I'll review it and merge it in if I can.

kylefarris commented 9 years ago

I apologize for the state that section of the code is in. It looks like the first bug was caused by the dreaded "this" vs "self" annoyance. Your PR addresses that, so that's good. But, you're right... the rest of the code in that block just doesn't make any sense the way I had it. Looks like I copied it from scan_dir and never got around to making it right for scan_files. It's rather embarrassing, actually.

Once I get your code merged in, I think it's time I wrote some tests for the module.

nicolaspeixoto commented 9 years ago

Yes Kyle, but no problem about it.

I would help you writing some tests if you want.

kylefarris commented 9 years ago

Thanks Nicolas. I'm writing some tests right now. I'll push them out to github once they're done and you can add any that you'd like to make it more comprehensive--up to you. I'll be using chai and mocha. After I have the tests up, I will publish the updated module to NPM and close this Issue.

kylefarris commented 9 years ago

Thanks for the pull request Nicolas! I've merged it in, written tests, and published a new version (0.7.0) to NPM. See the History file for details.

kylefarris commented 9 years ago

Just realized the modifications I made to the script now REQUIRE Node > 0.12. I'm in the process of fixing that now. The version and package file has been bumped to 0.7.1 to reflect the requirement of 0.12.

Sorry about that!