FileStorage error if user contains non-ascii characters

Bug #646019 reported by Gediminas Paulauskas
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
zope.app.publication
Confirmed
Undecided
Gediminas Paulauskas

Bug Description

FileStorage's TxnHeader has a 'user' field that is set by zope.app.publication to self.principal.id. However, if the principal id contains non-ascii characters, the transaction cannot be committed:

2010-09-23 16:23:41,238 http://localhost:7081/persons/%C5%BEivil%C4%97/preferences/index.html
Traceback (most recent call last):
  File "/home/menesis/.buildout/eggs/zope.publisher-3.12.4-py2.6.egg/zope/publisher/publish.py", line 137, in publish
    publication.afterCall(request, obj)
  File "/home/menesis/.buildout/eggs/zope.app.publication-3.11.1-py2.6.egg/zope/app/publication/browser.py", line 48, in afterCall
    super(BrowserPublication, self).afterCall(request, ob)
  File "/home/menesis/.buildout/eggs/zope.app.publication-3.11.1-py2.6.egg/zope/app/publication/zopepublication.py", line 213, in afterCall
    txn.commit()
  File "/home/menesis/.buildout/eggs/transaction-1.0.0-py2.6.egg/transaction/_transaction.py", line 322, in commit
    self._commitResources()
  File "/home/menesis/.buildout/eggs/transaction-1.0.0-py2.6.egg/transaction/_transaction.py", line 419, in _commitResources
    rm.tpc_vote(self)
  File "/home/menesis/.buildout/eggs/ZODB3-3.9.4-py2.6-linux-i686.egg/ZODB/Connection.py", line 767, in tpc_vote
    s = vote(transaction)
  File "/home/menesis/.buildout/eggs/ZODB3-3.9.4-py2.6-linux-i686.egg/ZODB/FileStorage/FileStorage.py", line 724, in tpc_vote
    self._file.write(h.asString())
  File "/home/menesis/.buildout/eggs/ZODB3-3.9.4-py2.6-linux-i686.egg/ZODB/FileStorage/format.py", line 278, in asString
    return "".join(map(str, [s, self.user, self.descr, self.ext]))
UnicodeEncodeError: 'ascii' codec can't encode character u'\u017e' in position 12: ordinal not in range(128)

This is the last problem preventing us from using unicode usernames (LP: #397610). The other issue was fixed in zope.publisher (LP: #131460).

descr field should also handle unicode strings.

Revision history for this message
Gediminas Paulauskas (menesis) wrote :

The ITransaction interface says

    user = zope.interface.Attribute(
        """A user name associated with the transaction.

        The format of the user name is defined by the application. The value
        is of Python type str. Storages record the user value, as meta-data,
        when a transaction commits...

that user is of type str. So the FileStorage implementation is correct in this sense. But I don't understand why it cannot be unicode.

If there is a reason, then re-target this bug to zope.app.publication that should encode the principal id.

Revision history for this message
Jim Fulton (jim-zope) wrote :

Perhaps it would be nice if the user field was unicode. <shrug> The
definition of this interface predates unicode in Python. This is
unlikely to change.

A more likely change is to drop these fields altogether and keep just
the extension data, which allows any picklable data.

I suggest changing zope.app.publication to store the user as extension
data.

BTW, while talking about transaction meta data, it would be nice if
zope.app.publication also recorded the host name and pid. :)

affects: zodb → zope.app.publication
Revision history for this message
Stephan Richter (srichter) wrote : Re: [Bug 646019] Re: FileStorage error if user contains non-ascii characters

On Thursday, September 23, 2010, Jim Fulton wrote:
> Perhaps it would be nice if the user field was unicode. <shrug> The
> definition of this interface predates unicode in Python. This is
> unlikely to change.
>
> A more likely change is to drop these fields altogether and keep just
> the extension data, which allows any picklable data.

What about simply using UTF-8 in zope.app.publication to encode and decode the
principal id. This would require the smallest code change.

Regards,
Stephan
--
Entrepreneur and Software Geek
Google me. "Zope Stephan Richter"

Revision history for this message
Jim Fulton (jim-zope) wrote :

On Thu, Sep 23, 2010 at 10:20 AM, Stephan Richter
<email address hidden> wrote:
> On Thursday, September 23, 2010, Jim Fulton wrote:
>> Perhaps it would be nice if the user field was unicode. <shrug> The
>> definition of this interface predates unicode in Python.  This is
>> unlikely to change.
>>
>> A more likely change is to drop these fields altogether and keep just
>> the extension data, which allows any picklable data.
>
> What about simply using UTF-8 in zope.app.publication to encode and decode the
> principal id. This would require the smallest code change.

I doubt it would be much smaller than:

   t.setUser(u) -> t.setExtendedInfo('user', u)

IIRC the publisher already uses extended info for the description/url.

Jim

--
Jim Fulton

Revision history for this message
Stephan Richter (srichter) wrote :

On Thursday, September 23, 2010, Jim Fulton wrote:
> I doubt it would be much smaller than:
>
> t.setUser(u) -> t.setExtendedInfo('user', u)
>
> IIRC the publisher already uses extended info for the description/url.

But wouldn't you have to potentially change code that depends on the user
being set in the user field? I think that's the real cost/risk.

Regards,
Stephan
--
Entrepreneur and Software Geek
Google me. "Zope Stephan Richter"

Revision history for this message
Jim Fulton (jim-zope) wrote :

On Thu, Sep 23, 2010 at 10:51 AM, Stephan Richter
<email address hidden> wrote:
> On Thursday, September 23, 2010, Jim Fulton wrote:
>> I doubt it would be much smaller than:
>>
>>    t.setUser(u) -> t.setExtendedInfo('user', u)
>>
>> IIRC the publisher already uses extended info for the description/url.
>
> But wouldn't you have to potentially change code that depends on the user
> being set in the user field?

Sure. Is there any code like that? The only code I can think of
would be zope 2's undo mechanism and zope 2 doesn't use
zope.app.publisher.

Jim

--
Jim Fulton

Revision history for this message
Marius Gedminas (mgedmin) wrote :

zodbbrowser uses IStorageUndoable's undoLog() which returns a list of dicts, and t.user is in those dicts as txn_info['user_name']. That would be easy to change.

For backwards compatibility why not use both?

   t.setUser(u.encode('UTF-8', 'replace')) # must be str
   t.setExtendedInfo('user', u) # unicode version

Changed in zope.app.publication:
status: New → Confirmed
assignee: nobody → Gediminas Paulauskas (menesis)
Changed in zope.app.publication:
assignee: Gediminas Paulauskas (menesis) → nobody
assignee: nobody → Gediminas Paulauskas (menesis)
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Related questions

Remote bug watches

Bug watches keep track of this bug in other bug trackers.