Edgewall Software
Modify

Opened 14 years ago

Closed 14 years ago

Last modified 13 years ago

#2284 closed defect (fixed)

Delete page exhibits same behaviour as delete version

Reported by: trac-form@… Owned by: Christopher Lenz
Priority: normal Milestone: 0.9.1
Component: wiki system Version: 0.9
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:

Description

When I "delete page", it displays the delete page confirmation page, but only deletes the current version.

Delete version URL:

http://trac-hacks.swapoff.org/wiki/FooBar?action=delete&version=2&delete_version=Delete+this+version

Delete page URL:

http://trac-hacks.swapoff.org/wiki/FooBar?action=delete&version=2

If I manually remove the &version=2 from the URL, it deletes as expected.

Attachments (0)

Change History (7)

comment:1 by trac-form@…, 14 years ago

I modified trac/wiki/web_ui.py to raise a TracError if version is defined, and it did indeed raise. Matt suggested that perhaps I have an older version, but I just did a fresh checkout of trunk and reinstalled after rm -rf'ing /usr/lib/python-2.4/site-packages/trac. I also suspected it might be the Tags wiki.cs override, but I tried it with stock wiki.cs to no avail.

    def _do_delete(self, req, db, page):
        if page.readonly:
            req.perm.assert_permission('WIKI_ADMIN')
        else:
            req.perm.assert_permission('WIKI_DELETE')

        if req.args.has_key('cancel'):
            req.redirect(self.env.href.wiki(page.name))

        version = None
        if req.args.has_key('version'):
            version = int(req.args.get('version', 0))
            raise TracError("%i" % version)

comment:2 by trac-form@…, 14 years ago

Presumably this is occurring because the confirmation form has action="":

  <h1>Delete <a href="/wiki/FooBar">FooBar</a></h1>

  <form action="" method="post">
   <input type="hidden" name="action" value="delete" />
   <p><strong>Are you sure you want to completely delete this page?</strong><br />
   This is an irreversible operation.</p>
   <div class="buttons">
    <input type="submit" name="cancel" value="Cancel" />
    <input type="submit" value="Delete page" />
   </div>

  </form>

The URL has "version=2" in it, which then gets picked up by if req.args.has_key('version') as a sign to delete a specific version. The following code change fixes the issue:

  • trac/wiki/web_ui.py

     
    157157            req.redirect(self.env.href.wiki(page.name))
    158158
    159159        version = None
    160         if req.args.has_key('version'):
     160        if req.args.has_key('delete_version') and req.args.has_key('version'):
    161161            version = int(req.args.get('version', 0))
    162162
    163163        page.delete(version, db)

comment:3 by Matthew Good, 14 years ago

Owner: changed from Jonas Borgström to Matthew Good
Status: newassigned

Well, apparently with the CGI and FastCGI front-ends req.args contains only POST parameters when the request method is POST, but the mod_python front end is wrapping the request in a way that preserves the GET arguments as well. It seems like the cleanest thing would be to make mod_python conform to the CGI behavior. This means that POSTs wouldn't be able to access any GET parameters on the URL, though mixing the two should probably be avoided anyways and CGI and FastCGI seem to be working fine as they are now.

Unless there are any objections this patch should fix the mod_python frontend:

  • trac/web/modpython_frontend.py

     
    138138        a POST request. We work around this to provide the behaviour of cgi.py
    139139        here.
    140140        """
    141         class RequestWrapper(object):
    142             def __init__(self, req):
    143                 self.req = req
    144                 self.args = ''
    145             def __getattr__(self, name):
    146                 return getattr(self.req, name)
    147         util.FieldStorage.__init__(self, RequestWrapper(req), keep_blank_values=1)
     141        if req.method == 'POST':
     142            class RequestWrapper(object):
     143                def __init__(self, req):
     144                    self.req = req
     145                    self.args = ''
     146                def __getattr__(self, name):
     147                    return getattr(self.req, name)
     148            req = RequestWrapper(req)
     149        util.FieldStorage.__init__(self, req, keep_blank_values=1)
    148150
    149         # Populate FieldStorage with the original query string parameters, if
    150         # they aren't already defined through the request body
    151         if req.args:
    152             qsargs = []
    153             for pair in util.parse_qsl(req.args, 1):
    154                 if self.has_key(pair[0]):
    155                     continue
    156                 qsargs.append(util.Field(pair[0], StringIO(pair[1]),
    157                                          "text/plain", {}, None, {}))
    158             self.list += qsargs
    159 
    160151    def get(self, key, default=None):
    161152        # Work around a quirk with the ModPython FieldStorage class.
    162153        # Instances of a string subclass are returned instead of real

comment:4 by Christopher Lenz, 14 years ago

That behavior of the mod_python frontend was introduced in [1467] to fix #1370:

The problem here is that the FieldStorage implementation of mod_python behaves differently from the implementation in cgi.py. While the latter adds query-string parameters after POST body parameters, and does so only if the the parameter isn't already in the body, the mod_python implementation adds query string parameters first and unconditionally.

Looking at the Python cgi module (in 2.3), I think this analysis is correct, at least for forms that aren't multipart encoded:

    if environ['REQUEST_METHOD'] == 'POST':
        ctype, pdict = parse_header(environ['CONTENT_TYPE'])
        if ctype == 'multipart/form-data':
            return parse_multipart(fp, pdict)
        elif ctype == 'application/x-www-form-urlencoded':
            clength = int(environ['CONTENT_LENGTH'])
            if maxlen and clength > maxlen:
                raise ValueError, 'Maximum content length exceeded'
            qs = fp.read(clength)
        else:
            qs = ''                     # Unknown content-type
        if 'QUERY_STRING' in environ:
            if qs: qs = qs + '&'
            qs = qs + environ['QUERY_STRING']
        elif sys.argv[1:]:
            if qs: qs = qs + '&'
            qs = qs + sys.argv[1]
        environ['QUERY_STRING'] = qs    # XXX Shouldn't, really
    elif 'QUERY_STRING' in environ:
        qs = environ['QUERY_STRING']

So, for a form with the standard application/x-www-form-urlencoded encoding, the parameters in the request body are simply appended to the query string, which is then later parsed to populate the FieldStorage fields. Maybe this has been changed in Python 2.4?

comment:5 by Christopher Lenz, 14 years ago

Owner: changed from Matthew Good to Christopher Lenz
Status: assignednew

I'm going to fix this for now by simply correcting the action URL of the form.

comment:6 by Christopher Lenz, 14 years ago

Component: generalwiki
Status: newassigned

comment:7 by Christopher Lenz, 14 years ago

Resolution: fixed
Status: assignedclosed

Fixed in [2578] and [2579].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christopher Lenz.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Christopher Lenz 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.