Re: [mat-dev] Activity report

On 06/04/2013 10:23, intrigeri wrote:
> Hi,
>
> jvoisin wrote (05 Apr 2013 21:04:37 GMT) :
>> The MAT is now ported to Gtk3.
>
> Awesome! Congrats for all this good work :)
Arg. I pushed in the public repo instead of my private one :/
This is why everything was kind of "messed up", and why commits have
weird names. This won't happen again. But thank you for your feedbacks :)
>
> 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?
Urg, who (and why) would someone do this ?
Added to the TODO-list.
>
> 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.
This was not strictly a security issue, just a note to myself :/
>
> I'm glad you found a way to workaround the Cairo vs. Unicode bug.
> Congrats!
It's pretty ugly: I copy the file to a temp one with a nice (=without
weird chars) name, process it, and then copy it back.
>
> 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 :)
Again, my notes :/
>
> 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?
The MAT itself is designed for UTF-8. But the utf8 thing is because tar
wasn't designed to handle filenames with wild chars. I this is the less
worse solution. Does anyone else has an opinion about this issue ?
(See http://docs.python.org/2/library/tarfile.html#unicode-issues)

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.
Noted on the TODO-list.
>
>> +++ 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?
I don't think so, but this is a good idea: renamed.
>
> It's not clear to me why tarfile.py is removed in the commit that adds
> initial Gtk3 support.
Because tarfile was an embedded copy of python2.7's tarfile, and since I
moved from gtk2 to gtk3, I decided to drop the support of python2.6.
>

>> +        ( '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?
Yes :/
But the good news is that the nautilus extension is getting closer
thanks to the Gtk3 port !
>
> $ ./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?
They are non-fatal, except for PDF processing.
The portage to GTK3 introduced a new dependency: gir1.2-poppler-0.18,
replacing the old python-poppler one.
>
> Tar is still listed in the mat-gui supported file formats dialog,
> while PDF is not.
I removed tar.gz, but PDF was already present.
>
> 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 !
I replaced the "print" with proper logging instruction.
>
> 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.)
Weird, but I think that I designed something wrong.
I don't think that I should compare translated strings,
but constants instead. Maybe an associative array ?
Anyway, it's added to the TODO-list.
>
> Time to setup a bug / ticket tracker, perhaps? ;)
Since I don't think that I can fix bug as quickly as they are popping in
my mailbox, this might be a good idea :)

> (If in doubt, I recommend asking a Redmine project at
> https://labs.riseup.net/code/.)
Since they are already hosting Tail's one, this seems like a good idea.
I opened a ticket, thank you for the wise suggestion.