Re: [mat-dev] Activity report

Hi,

jvoisin wrote (05 Apr 2013 21:04:37 GMT) :
> The MAT is now ported to Gtk3.

Awesome! Congrats for all this good work :)

Here's a very quick review of the recent changes.

> +        def clean_path(url):
> +            '''
> +                Since the dragged urls are ugly,
> +                we need to process them
> +            '''
> +            url = urllib2.unquote(url)  # unescape stupid chars
> +            url = url.replace('\n', '')

Does this mean files with "\n" in their names are not supported?

About commits such as cdf8a1 that contain additional unrelated and
undocumented changes: I suggest learning about `git add -p' :)

About commit 16d92f16 "Fix a security issue in the cli testsuite":
I'm sorry, I don't get how this is a security issue. Could you please
clarify? And, if it's really a security issue, please make it clear
what released versions of the MAT are affected, and perhaps request
a CVE number so that vendors are aware and can act accordingly.

I'm glad you found a way to workaround the Cairo vs. Unicode bug.
Congrats!

About commit b2ff71a ("Fix unit tests"): first, unit tests are a great
way to discover bugs, but I guess this commit really is about fixing
a bug, not the unit tests, isn't it? Please write commit messages that
document the why & how of the changes :)

Anyway, still in this commit:

> -        tarin = tarfile.open(self.filename, 'r' + self.compression)
> -        tarout = tarfile.open(self.output, 'w' + self.compression)
> +        tarin = tarfile.open(self.filename, 'r' + self.compression, encoding='utf-8')
> +        tarout = tarfile.open(self.output, 'w' + self.compression, encoding='utf-8')

I'm too lazy to go read tarfile documentation, sorry. Does this mean
utf-8 encoding is assumed for some part of the input, e.g. for the
filename? If this is so, then what happens if the filename is encoded
differently? It might be time to add test files whose name is
encoded differently.

> +++ b/MAT/mutagenpowered.py
> [...]
> class MutagenStripper(parser.GenericParser):

Just curious: hasn't Python any stricter best practices or conventions
wrt. how to name files that contain classes, accordingly to these
classes' name?

It's not clear to me why tarfile.py is removed in the commit that adds
initial Gtk3 support.

> +        ( 'share/nautilus-python/extensions', ['nautilus/nautilus-mat.py'] ),

... in the commit that adds Gtk3 support, while the Nautilus extension
is not in this Git tree. Mistake?

$ ./mat -c data/mat.png
ERROR:root:Could not find any typelib for Poppler
ERROR:root:Could not find any typelib for Poppler
Unable to import Poppler
[+] data/mat.png is not clean

Is it expected that such import errors are non-fatal?

Tar is still listed in the mat-gui supported file formats dialog,
while PDF is not.

I can clean this file with the CLI:

$ ./mat ~/tails/git/features/images/PolicyKitAuthPrompt.png
ERROR:root:Could not find any typelib for Poppler
ERROR:root:Could not find any typelib for Poppler
Unable to import Poppler
[+] Cleaning /home/intrigeri/cyber/tails/git/features/images/PolicyKitAuthPrompt.png
/home/intrigeri/cyber/tails/git/features/images/PolicyKitAuthPrompt.png cleaned !

But I get a warning in the GUI:

$ ./mat-gui:588: UnicodeWarning: Unicode unequal comparison failed to convert both arguments to Unicode - interpreting them as being unequal
if self.force or self.liststore[line][4] != _('Clean'):

(running with French locales.)

Time to setup a bug / ticket tracker, perhaps? ;)
(If in doubt, I recommend asking a Redmine project at
https://labs.riseup.net/code/.)

Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc