[patch] existing tickets are opened via /newticket URL

[pkou at ua.fm]

It is possible to open existing tickets using /newticket URL.

Sample: http://tracsite/newticket?id=1 will open ticket #1

Possible fix:

web_ui.py

@@ -115 +115 @@
             return True
 
     def process_request(self, req):
-        if 'id' in req.args:
+        if re.match(r'/ticket/([0-9]+)$', req.path_info) is not None:
             return self._process_ticket_request(req)
         return self._process_newticket_request(req)

Cf. with match_request also.

[Christian Boos]

A req.path_info.startswith('/newticket/') should also do.

[Christian Boos]

Fixed in r6014.

[Noah Kantrowitz]

I would rather see this redirect to the correct /ticket page than just show an error. Thoughts?

[Christian Boos]

I think redirects should rather be used for "normalizing" different valid URLs to the same canonical URL (like we do for a Wiki page with name ending with "/", for example).

Here (/newticket?id=1) it's clearly an error and not an alternative "valid" URL for /ticket/1.

[hyuga <hyugaricdeau@…>]

Replying to cboos:

I think redirects should rather be used for "normalizing" different valid URLs to the same canonical URL (like we do for a Wiki page with name ending with "/", for example).

Here (/newticket?id=1) it's clearly an error and not an alternative "valid" URL for /ticket/1.

I agree. Wouldn't really want users thinking /newticket?id=1 is at all valid. I could see that leading to bad things.

[Christian Boos]

Closing again, /newticket?id=1 is definitely a bogus URL and should be reported as such.

Now, we could eventually reconsider the problem and allow the creation of tickets with an explicit id if they don't exist, but that's a different issue and brings some new concerns (would work just fine for deleted tickets, but what about ids that have simply not yet been reached?).

[Christian Boos]

Replying to cboos:

Closing again, /newticket?id=1 is definitely a bogus URL and should be reported as such.

I've reconsidered this after r6120,6122 made the issue go away: adding back the check and raising an error would only have made the code more complex and goes in the direction of strictly checking all the URL parameters, which I'm not sure is a good thing to do (thinking about extensions). So better ignore that extra id since now it is harmless (see r6565).