Edgewall Software

Ticket #4087 (closed defect: fixed)

Opened 19 months ago

Last modified 3 months ago

Unable to attach binaries when the SpamFilter is active

Reported by: Markus Gritsch Owned by: cboos
Priority: highest Milestone: not applicable
Component: plugin/spamfilter Version: 0.10
Severity: critical Keywords: postgresql binary unicode
Cc: cramm0@…

Description

I wanted to attach a precompiled version of SilverCity to TracSyntaxColoring because SilverCity 0.9.5 is not available for Python 2.4 from SourceForge. The file I tried to upload has the name SilverCity-0.9.5.win32-py2.4.exe

I got the following traceback:

Traceback (most recent call last):
  File "/usr/lib/python2.4/site-packages/trac/web/main.py", line 381, in dispatch_request
    dispatcher.dispatch(req)
  File "/usr/lib/python2.4/site-packages/trac/web/main.py", line 231, in dispatch
    resp = chosen_handler.process_request(req)
  File "/usr/lib/python2.4/site-packages/trac/attachment.py", line 361, in process_request
    self._do_save(req, attachment)
  File "/usr/lib/python2.4/site-packages/trac/attachment.py", line 474, in _do_save
    for field, message in manipulator.validate_attachment(req, attachment):
  File "build/bdist.linux-x86_64/egg/tracspamfilter/adapters.py", line 130, in validate_attachment
  File "build/bdist.linux-x86_64/egg/tracspamfilter/api.py", line 131, in test
  File "build/bdist.linux-x86_64/egg/tracspamfilter/model.py", line 119, in insert
  File "/usr/lib/python2.4/site-packages/trac/db/util.py", line 47, in execute
    return self.cursor.execute(sql_escape_percent(sql), args)
  File "/usr/lib/python2.4/site-packages/trac/db/util.py", line 47, in execute
    return self.cursor.execute(sql_escape_percent(sql), args)
ProgrammingError: current transaction is aborted, commands ignored until end of transaction block

Attachments

spamfilter_log-base64content-r5329.diff (5.7 kB) - added by cboos 13 months ago.
Encode content to base64 before storing it in the db
SilverCity-0.9.5.win32-py2.4.exe (187.4 kB) - added by cboos 12 months ago.
test binary attachment that originally failed (I'm not logged in)
test4087.txt (1.0 kB) - added by anonymous 12 months ago.
Make sure the fix works for files that escape the is_binary test.
SilverCity-0.9.5.win32-py2.4.2.exe (187.4 kB) - added by anonymous 8 months ago.
check this is working one more time…

Change History

  Changed 19 months ago by cboos

  • owner changed from jonas to mgood
  • priority changed from normal to highest
  • component changed from wiki to spamfilter
  • severity changed from normal to major

Yes, this has been noted a couple of time. There's an issue with the latest SpamFilter and attachments in 0.10/0.10-stable.

  Changed 18 months ago by cboos

From #4134, it looks like only binary attachments are affected, as the patches were successfully attached there by the same user.

I've looked a bit at the code, but there's nothing obvious... the content extracted from the file was transformed into an unicode string using to_unicode, so this must always succeed (and there's no unicode error anyway here).

Could it be that PostgreSQL doesn't allow binary content in string fields?

  Changed 18 months ago by anonymous

That very well might be the case. See http://www.postgresql.org/docs/8.1/interactive/datatype-binary.html

Character strings disallow zero octets, and also disallow any other octet values and sequences of octet values that are invalid according to the database's selected character set encoding.

  Changed 18 months ago by cboos

Ok, I see. So either the SpamFilter model should adopt this bytea binary datatype for the "content" column, or the attachment content should itself be "binary escaped" before being stored in that table.

in reply to: ↑ description ; follow-up: ↓ 11   Changed 18 months ago by mikolaj

Replying to Markus Gritsch:

I wanted to attach a precompiled version of SilverCity to TracSyntaxColoring because SilverCity 0.9.5 is not available for Python 2.4 from SourceForge. The file I tried to upload has the name SilverCity-0.9.5.win32-py2.4.exe

Is it possible that you put this precompiled file in some other plase, I mean any freeware hosting so we (dummy people ;-) ) could download it?

Best regards Mikolaj

  Changed 18 months ago by Noah Kantrowitz (coderanger) <coderanger@…>

PyPI was created for this purpose. You can ask the SilverCity team to either list their package on PyPI, or allow you do to do so.

  Changed 18 months ago by cboos

#4211 was marked as duplicate. Apparently, there's a similar issue with pysqlite1 as well.

  Changed 18 months ago by anonymous

and about the error on #4211, it was not because of a binary, but not sure if this has something to do.

  Changed 18 months ago by cmlenz

  • status changed from new to closed
  • resolution set to fixed

[4340] changed the attachment filter adapter to ignore binary attachment data (although the comment and author fields are still checked).

follow-up: ↓ 12   Changed 18 months ago by cboos

The heuristic is_binary used in [4340] is probably not enough:

  • only checks the first 1000 bytes
  • only checks for '\0', which according to comment:3 is not the only byte to be rejected as binary

in reply to: ↑ 5   Changed 18 months ago by Markus Gritsch < >

Replying to mikolaj:

Is it possible that you put this precompiled file in some other plase, I mean any freeware hosting so we (dummy people ;-) ) could download it?

SilverCity 0.9.7 was released the day before yesterday and it seems that it works also well with Trac. It is available precompiled for Python versions from 2.0 to 2.5.

in reply to: ↑ 10   Changed 18 months ago by cboos

  • status changed from closed to reopened
  • resolution deleted

Replying to cboos:

The heuristic is_binary used in [4340] is probably not enough:

Happened again today on #4257. Even if the SpamFilter on t.e.o was not actually updated to [4340], I think it's just a matter of time this problem happens again, for the reasons already given above.

  Changed 17 months ago by cboos

  • severity changed from major to critical

#4440 shows that the problem is beyond binary content. The Unicode content also nearly systematically triggers this exception, which is therefore a very serious.

  Changed 17 months ago by cboos

  • keywords postgresql binary unicode added
  • milestone set to none

Note that the backtrace is easily reproduced on t.e.o. when pasting the TracUnicode page inside a wiki page (as anonymous, of course).

Traceback (most recent call last):
...
  File "/usr/lib/python2.4/site-packages/trac/wiki/web_ui.py", line 231, in _do_save
    for field, message in manipulator.validate_wiki_page(req, page):
  File "build/bdist.linux-x86_64/egg/tracspamfilter/adapters.py", line 98, in validate_wiki_page
  File "build/bdist.linux-x86_64/egg/tracspamfilter/api.py", line 131, in test
  File "build/bdist.linux-x86_64/egg/tracspamfilter/model.py", line 119, in insert
  File "/usr/lib/python2.4/site-packages/trac/db/util.py", line 50, in execute
    return self.cursor.execute(sql_escape_percent(sql), args)
  File "/usr/lib/python2.4/site-packages/trac/db/util.py", line 50, in execute
    return self.cursor.execute(sql_escape_percent(sql), args)
ProgrammingError: current transaction is aborted, commands ignored until end of transaction block

Jonas, it seems that this operation works in other installations (e.g. s0undtech tried to reproduce that but it worked for him). Is there anything special about this table/column on t.e.o, or in the SpamFilter code?

follow-up: ↓ 16   Changed 16 months ago by cboos

  • summary changed from Unable to attach SilverCity-0.9.5.win32-py2.4.exe to Wiki site to Unable to attach binaries when the SpamFilter is active

#4562 also exhibited the problem.

A simple solution would be to encode the content as base64, for example.

in reply to: ↑ 15   Changed 16 months ago by eblot

Replying to cboos:

A simple solution would be to encode the content as base64, for example.

I just got the same issue on TracHacks (0.10.3) while attempting to attach a PNG file and being logged in..., four time in a row. I still can't attach the image file ;-(

  Changed 16 months ago by eblot

#4603 has been marked as a duplicate.

  Changed 15 months ago by cboos

So actually this is even worse than just a problem with binaries...

It chokes even for "normal" unicode characters in ticket descriptions.

See #4718 for an example.

  Changed 15 months ago by cboos

#4858 shows that it's not only this Trac...

There, the error message was:

TypeError: execute() argument 1 must be string without null bytes, not str

  Changed 15 months ago by techtonik <techtonik@…>

#4992 is the same - trying to edit unicode page yields error and attaching the file with edited content to the ticket results in the same problem. Seems like this spamfilter tries to analyze every kind of content and chokes on certain character sequences if even .PNG is affected.

  Changed 13 months ago by cboos

#5250 marked as duplicate, raising the priority... oops, it's already "highest" priority ;-)

Changed 13 months ago by cboos

Encode content to base64 before storing it in the db

  Changed 13 months ago by cboos

  • owner changed from mgood to cboos
  • status changed from reopened to new

jonas, can you please try out spamfilter_log-base64content-r5329.diff ?

If this fixes the issues we have on t.e.o, I'll commit it.

  Changed 12 months ago by Ramiro Morales <cramm0@…>

  • cc cramm0@… added

Changed 12 months ago by cboos

test binary attachment that originally failed (I'm not logged in)

Changed 12 months ago by anonymous

Make sure the fix works for files that escape the is_binary test.

  Changed 12 months ago by cboos

  • status changed from new to closed
  • resolution set to fixed

Above fix tested on t.e.o and applied in r5390.

Changed 8 months ago by anonymous

check this is working one more time...

  Changed 3 months ago by cboos

#5115 was closed as duplicate -

ProgrammingError: invalid byte sequence for encoding "UTF8": 0xe52031

received when retrieving saved content from the db. How that sequence of bytes managed to go in the db without triggering an error at insert time is a bit weird though.

Add/Change #4087 (Unable to attach binaries when the SpamFilter is active)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
 
Note: See TracTickets for help on using tickets.