Edgewall Software
Modify

Opened 10 years ago

Closed 10 years ago

#11518 closed defect (fixed)

UnicodeEncodeError: 'decimal' codec can't encode character u'\u8b70' in position 2: invalid decimal Unicode string

Reported by: hirano Owned by: Ryan J Ollos
Priority: normal Milestone: 0.12.6
Component: wiki system Version: 0.12.4
Severity: normal Keywords:
Cc: Branch:
Release Notes:

ResourceNotFound error is raised when wiki page version is invalid.

API Changes:
Internal Changes:

Description

How to Reproduce

While doing a GET operation on /wiki/3月4日 開発会議 議事録, Trac issued an internal error.

(please provide additional details here)

Request parameters:

{'page': u'3\u67084\u65e5\u3000\u958b\u767a\u4f1a\u8b70\u3000\u8b70\u4e8b\u9332',
 'version': u'1/\u8b70\u4e8b\u9332'}

User agent: Mozilla/5.0 (Windows NT 6.1; rv:27.0) Gecko/20100101 Firefox/27.0

System Information

Trac 0.12.4.ja1
Babel 0.9.5
CustomFieldAdmin 0.2.8
Docutils 0.7
Genshi 0.6 (without speedups)
mod_wsgi 3.3 (WSGIProcessGroup WSGIApplicationGroup %{GLOBAL})
Pygments 1.3.1
pysqlite 2.4.1
Python 2.6.6 (r266:84297, Aug 24 2010, 18:46:32) [MSC v.1500 32 bit (Intel)]
RPC 1.1.2-r9970
setuptools 0.6c11
SQLite 3.5.9
Subversion 1.6.17 (r1128011)
jQuery 1.4.4

Enabled Plugins

advancedticketworkflowplugin 0.11dev-r9962
batchmodify 0.8.0a1-trac0.12
blockdiagplugin 0.5.1
exceldownloadplugin 0.12.0.1
footnotemacro 1.03
hudsontracplus 0.4
iniadmin 0.2
lightningtheme 1.0
newwikipagebutton 1.01
privatewikis 1.0.0
reportincludeplugin 0.5-snapshot
svnauthzadminplugin 0.1.2.-moved.to.trac.0.11-
ticket-clone Rev
ticketimport 0.8
trac-jsgantt 0.2.0-ja
tracaccountmanager 0.3
tracaddcommentmacro 0.3
tracautowikify 0.1
tracavatarplugin 0.3
traccompleteuserplugin 0.4
traccustomfieldadmin 0.2.8
tracdatefield 1.0.1
tracdiscussion 0.8-r9877
tracdragdrop 0.12.0.10
tracflashembedmacro 0.95rc1
tracganttcalendarplugin 0.5-r801
tracmacostheme 1.0.3
tracmacropost 0.2
tracnav 4.2.dev
tracquerychart 0.2.3
tracsectioneditplugin 0.1
tracsubticketsplugin 0.1.1.dev-20130429
tracthemeengine 2.0.1
tractocmacro 11.0.0.3
tracusermanagerplugin 0.4r5520
tracwysiwyg 0.12.0.4-r9676
tracxdocview 0.1
tracxmlrpc 1.1.2-r9970
workfloweditorplugin 1.1.5

Python Traceback

Traceback (most recent call last):
  File "C:\TracLight\python\lib\site-packages\trac-0.12.4.ja1-py2.6.egg\trac\web\main.py", line 522, in _dispatch_request
    dispatcher.dispatch(req)
  File "C:\TracLight\python\lib\site-packages\trac-0.12.4.ja1-py2.6.egg\trac\web\main.py", line 243, in dispatch
    resp = chosen_handler.process_request(req)
  File "C:\TracLight\python\lib\site-packages\trac-0.12.4.ja1-py2.6.egg\trac\wiki\web_ui.py", line 119, in process_request
    versioned_page = WikiPage(self.env, pagename, version=version)
  File "C:\TracLight\python\lib\site-packages\trac-0.12.4.ja1-py2.6.egg\trac\wiki\model.py", line 40, in __init__
    version = int(version) # must be a number or None
UnicodeEncodeError: 'decimal' codec can't encode character u'\u8b70' in position 2: invalid decimal Unicode string

Attachments (0)

Change History (15)

comment:1 by Jun Omae, 10 years ago

Component: generalwiki system
Milestone: next-minor-0.12.x
Version: 0.12.4

Thanks for reporting. Reproduced with the following on 0.12-stable.

wiki:WikiStart@1abc

It seems that a Trac link like [3月4日 開発会議 議事録@1/議事録] leads the issue.

comment:2 by Jun Omae, 10 years ago

I found #8893 has been closed as worksforme.

However, I think we should raise a ResourceNotFound with the following message as same one at tags/trac-0.12.5/trac/wiki/web_ui.py@:124-126#L123 for wiki:WikiStart@1abc.

No version "1abc" for Wiki page "WikiStart"

comment:3 by Ryan J Ollos, 10 years ago

Milestone: next-minor-0.12.x0.12.6

Proposed change in [0f58d52b/rjollos.git].

Additionally, some proposed changes for the trunk in [130fe4e8/rjollos.git] and [ccb53f36/rjollos.git].

in reply to:  3 ; comment:4 by Peter Suter, 10 years ago

Replying to rjollos:

Proposed change in [0f58d52b/rjollos.git].

(How is #11434 related?)

[130fe4e8/rjollos.git]

What should happen if version and resource.version do not match? The alternatives are:

  1. version has precedence over revision.version.
  2. revision.version has precedence over version.
  3. Throw an exception.

The current behavior is inconsistent (page.resource.version != page.version) but the most important functionality (page.text etc.) follow 1.

Your proposed changes fix the inconsistency, and follow 2.

I think I would prefer 3., or consistently 1. (Keep the most important functionality the same as before. Also explicit specification of version could be interpreted to signal intent to override resource.version.)

[ccb53f36/rjollos.git].

This breaks compatibility for WikiPage(env, name='bla').

in reply to:  4 ; comment:5 by Ryan J Ollos, 10 years ago

Replying to psuter:

Replying to rjollos:

Proposed change in [0f58d52b/rjollos.git].

(How is #11434 related?)

The previous ErrorPageValidation test was failing because it relied on navigation to /wiki/WikiStart?version=bug raising an internal error.

The test case for #11434 contains a class that raises an exception when navigating to the URL /raise-exception. The test for #11434 is the same as what's needed for this ticket, the only difference being that the Create ticket button directs to a different location depending on the frame in which the exception was raised. Therefore I handle both scenarios in a single test case.

This breaks compatibility for WikiPage(env, name='bla').

I guess I should revert that.

in reply to:  5 comment:6 by Peter Suter, 10 years ago

Adding a docstring might be a nice alternative to renaming.

comment:7 by Ryan J Ollos, 10 years ago

Owner: set to Ryan J Ollos
Release Notes: modified (diff)
Status: newassigned

Primary fix committed to 0.12-stable in [12585], merged in [12586:12587] (TracLinks are not linked due to problem reported in comment:14:ticket:11288).

I'm giving more thought to comment:4 and I'll eventually post some changes based on trunk for review.

comment:8 by Jun Omae, 10 years ago

The r12585 leads inconsistency for fetching non-existent versions.

Python 2.4.3 (#1, Jan  9 2013, 06:49:54)
[GCC 4.1.2 20080704 (Red Hat 4.1.2-54)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from trac import __version__
>>> __version__
'0.12.6dev'
>>> from trac.env import Environment
>>> from trac.wiki.model import WikiPage
>>> env = Environment('/home/jun66j5/var/trac/0.12')
>>> page = WikiPage(env, 'WikiStart')
>>> page.version
2
>>> page = WikiPage(env, 'WikiStart', 2)
>>> page.version
2
>>> page = WikiPage(env, 'WikiStart', 99999999)
>>> page.exists
False
>>> page.version
0
>>> page = WikiPage(env, 'WikiStart', '1abc')
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "trac/wiki/model.py", line 41, in __init__
    num=version, name=name))
trac.resource.ResourceNotFound: No version "1abc" for Wiki page "WikiStart"
Version 0, edited 10 years ago by Jun Omae (next)

comment:9 by Ryan J Ollos, 10 years ago

The inconsistency existed in r12584 as well, we just raise a different exception now.

Python 2.7.4 (default, Sep 26 2013, 03:20:26) 
[GCC 4.7.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from trac import __version__
>>> __version__
'0.12.6dev-r0'
>>> from trac.env import Environment
>>> from trac.wiki.model import WikiPage
>>> env = Environment('/home/user/Workspace/t11518/tracdev')
>>> page = WikiPage(env, 'WikiStart')
>>> page.version
2
>>> page = WikiPage(env, 'WikiStart', 2)
>>> page.version
2
>>> page = WikiPage(env, 'WikiStart', 99999999)
>>> page.exists
False
>>> page.version
0
>>> page = WikiPage(env, 'WikiStart', '1abc')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "trac/wiki/model.py", line 40, in __init__
    version = int(version) # must be a number or None
ValueError: invalid literal for int() with base 10: '1abc'
Last edited 10 years ago by Ryan J Ollos (previous) (diff)

in reply to:  9 comment:10 by Jun Omae, 10 years ago

The inconsistency existed in r12584 as well, we just raise a different exception now.

I don't think WikiPage should raise a ResourceNotFound exception. That would be API Changes. That ValueError is just a defect.

Last edited 10 years ago by Jun Omae (previous) (diff)

comment:11 by Ryan J Ollos, 10 years ago

We can move the invalid version trapping to trac.wiki.web_ui if you'd prefer.

  • trac/wiki/model.py

    diff --git a/trac/wiki/model.py b/trac/wiki/model.py
    index ef22e59..0ad3c2e 100644
    a b class WikiPage(object):  
    3232
    3333    def __init__(self, env, name=None, version=None, db=None):
    3434        self.env = env
    35         if version:
    36             try:
    37                 version = int(version)  # must be a number or None
    38             except ValueError:
    39                raise ResourceNotFound(
    40                     _('No version "%(num)s" for Wiki page "%(name)s"',
    41                       num=version, name=name))
    4235        if isinstance(name, Resource):
    4336            self.resource = name
    4437            name = self.resource.id
    4538        else:
     39            if version:
     40                version = int(version) # must be a number or None
    4641            self.resource = Resource('wiki', name, version)
    4742        self.name = name
    4843        if name:
  • trac/wiki/web_ui.py

    diff --git a/trac/wiki/web_ui.py b/trac/wiki/web_ui.py
    index f68b43a..575bde3 100644
    a b class WikiModule(Component):  
    114114            raise TracError(_("Invalid Wiki page name '%(name)s'",
    115115                              name=pagename))
    116116
     117        if version is not None:
     118            try:
     119                version = int(version)
     120            except (ValueError, TypeError):
     121               raise ResourceNotFound(
     122                    _('No version "%(num)s" for Wiki page "%(name)s"',
     123                      num=version, name=pagename))
     124
    117125        page = WikiPage(self.env, pagename)
    118126        versioned_page = WikiPage(self.env, pagename, version=version)
Last edited 10 years ago by Ryan J Ollos (previous) (diff)

comment:12 by Ryan J Ollos, 10 years ago

Proposed changes in log:rjollos.git:t11518.2.

comment:13 by Ryan J Ollos, 10 years ago

Some changes to the test cases as well: [8facff75/rjollos.git].

in reply to:  4 comment:14 by Ryan J Ollos, 10 years ago

Replying to psuter:

What should happen if version and resource.version do not match?

I think I would prefer 3., or consistently 1. (Keep the most important functionality the same as before. Also explicit specification of version could be interpreted to signal intent to override resource.version.)

I think you are right about consistently using (1). I've opened #11544 to address the issue.

comment:15 by Ryan J Ollos, 10 years ago

Resolution: fixed
Status: assignedclosed

Changes from log:rjollos.git:t11518.2 committed to 0.12-stable in [12597], merged in [12598:12599].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.