Edgewall Software
Modify

Opened 8 years ago

Closed 7 years ago

Last modified 6 years ago

#4087 closed defect (fixed)

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@…
Release Notes:
API Changes:

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 (4)

spamfilter_log-base64content-r5329.diff (5.7 KB) - added by cboos 7 years 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 7 years ago.
test binary attachment that originally failed (I'm not logged in)
test4087.txt (1022 bytes) - added by anonymous 7 years 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 7 years ago.
check this is working one more time…

Download all attachments as: .zip

Change History (29)

comment:1 Changed 8 years ago by cboos

  • Component changed from wiki to spamfilter
  • Owner changed from jonas to mgood
  • Priority changed from normal to highest
  • 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.

comment:2 Changed 8 years 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?

comment:3 Changed 8 years 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.

comment:4 Changed 8 years 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.

comment:5 in reply to: ↑ description ; follow-up: Changed 8 years 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

comment:6 Changed 8 years 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.

comment:7 Changed 8 years ago by cboos

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

comment:8 Changed 8 years 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.

comment:9 Changed 8 years ago by cmlenz

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:10 follow-up: Changed 8 years 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

comment:11 in reply to: ↑ 5 Changed 8 years 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.

comment:12 in reply to: ↑ 10 Changed 8 years ago by cboos

  • Resolution fixed deleted
  • Status changed from closed to reopened

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.

comment:13 Changed 8 years 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.

comment:14 Changed 8 years 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?

comment:15 follow-up: Changed 8 years 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.

comment:16 in reply to: ↑ 15 Changed 8 years 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 ;-(

comment:17 Changed 8 years ago by eblot

#4603 has been marked as a duplicate.

comment:18 Changed 7 years 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.

comment:19 Changed 7 years 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

comment:20 Changed 7 years 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.

comment:21 Changed 7 years ago by cboos

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

Changed 7 years ago by cboos

Encode content to base64 before storing it in the db

comment:22 Changed 7 years 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.

comment:23 Changed 7 years ago by Ramiro Morales <cramm0@…>

  • Cc cramm0@… added

Changed 7 years ago by cboos

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

Changed 7 years ago by anonymous

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

comment:24 Changed 7 years ago by cboos

  • Resolution set to fixed
  • Status changed from new to closed

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

Changed 7 years ago by anonymous

check this is working one more time…

comment:25 Changed 6 years 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 Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed The owner will remain cboos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from cboos to the specified user.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.