[OH-Dev] The problem with /do URLs

[Asheesh Laroia]

Hey all,

We've recently had some "fun" problems with certain URL patterns in 
urls.py. Since this process has taught me something about how to construct 
reliable URL patterns in Django, I wanted to take a quick moment to 
explain the problem, and offer a solution.

TL;DR: URL routes for 'name' attributes must permit all characters. For 
simplicity, this sometimes means that corresponding views must accept 
POST, rather than having the POST go to a separate URL.

== Background ==

The URLs for editing bug tracker information, /customs/ , used to look 
kind of like this:

(r'^customs/edit/(\w*)/([^/]+)$')

In English, rather than regex-speak, this means: "customs/edit/" followed 
by any word characters, followed by a slash, followed by any non-slash 
characters.

That meant that if a tracker name was created, such as "Debian GNU/Linux", 
that had a slash in it, there was no URL possible that could represent it!

I let that slip in with Jack's commit series that created the /customs/ 
URL paths. We would see five-ish crashes a week where someone requested a 
page that would try to create a URL that was impossible to create.

For a while, I had no idea what the problem was. Now I understand it's 
that the backend wanted to generate URLs that the route could not express.

== "Fix" ==

So I pushed a "fix", which was also wrong: 
fc9669db3564696163aafee150b4a0b4c6047d32

This fix just makes the URL paths be permissive to all characters.

But this conflicted with another idiom in the URLs. For example:

(r'^customs/edit/(\w*)/([^/]+)/do$')

This is the same route as above, but with "/do" at the end. We would have 
POST forms submit to this URL. But now the "/do" would be interpreted as 
part of the project name, so you'd actually get a 404 when you try to POST 
to the /do version of a URL. That's because it would match the URL that 
expects a GET, and then that view would say, "No such tracker by that 
name!"

== Adrian's fix ==

Adrian lately has been re-ordering the routes so the /do route appears 
above the views that don't have /do at the end.

However, this introduces a new kind of oddity, where if (by chance) a 
tracker_name ends with /do you'd still get an error.

== Even better fix? (vaporware) ==

Instead, for URLs where user data goes at the end of the URL, we shouldn't 
suffix anything to it in a way that could be ambiguous. (We also shouldn't 
prefix anything to it that could be ambiguously parsed.)

I've begun to commit some changes to implement that, but I'm going to have 
to leave that behind for a while. If someone else wants to take up the 
charge, that'd be amazing.

https://github.com/paulproteus/oh-mainline/tree/cleanup/remove-slash-do-from-customs-views 
is the Github URL of my branch.

In order for my bad fix to work, I had to revert a test. Reverting the 
test is what enabled the insanity that Adrian has only just fixed. So I 
re-added that test in 
https://github.com/paulproteus/oh-mainline/commit/15ffcb8462c76941cc8155fdb3d2e9b818641d32

It's probably not urgent, but that's the way I think it would be even 
better if things were!

-- Asheesh.