Rocket: Adding ? to any GET route that doesn't take a form produces a 404

[lilyball]

If I take a normal GET route that has no form, such as

#[get("/")]

and I stick a ? on the end of the URL, e.g. http://localhost:8000/? then I get a 404 instead of the route. This is rather surprising. I would not expect adding an empty query string would produce a 404. Heck, if the URL doesn't take a query string, I would expect it to just ignore any query string I passed (even non-empty ones).

[SergioBenitez]

That's not the way routing works, and it's certainly not what I would expect. If we did things the way you propose, then routes declared with and without query strings would match against the same requests; how would we disambiguate? We could complicate the matching rules, but that's a bad route to take, and likely a slow one.

Requests with queries only match routes with queries, and likewise requests without queries only match routes without queries. It might make sense to say that a request with an empty query string is the same as one without a query string, but I'm not convinced that's a good path to take.

[lilyball]

What I'm thinking is that, if I provide a query string, and no route matches with the query string, then it falls back to matching as if I didn't have a query string.

That said, I don't feel particularly strongly that /foo?bar should necessarily match /foo (though it would be nice). But I still think it's surprising that /foo? wouldn't fall back to matching /foo.

The reason why I'm suggesting this is because I can't think of any other site where adding ? onto a URL suddenly produces a 404.

[SergioBenitez]

How do you get an empty query string in the first place? That presumably wouldn't happen in a properly constructed application.

I'll think about whether Rocket should consider an empty query string to be the same as no query string; maybe an RFC somewhere has advice on this.

[lilyball]

I got it in this case because I had tossed in a <button> to trigger a GET request, and it put the empty query string on there. What I had looked like

  <form action="/foo/{{ foo.id }}/edit">
    <button type="submit">Edit</button>
  </form>

I've since changed it to just a bare link.

[lilyball]

If it helps, the way I'm thinking about this is that my URL identifies my resource, and a 404 means the resource doesn't exist, so if /foo/123/edit exists, then /foo/123/edit? should also exist, as should /foo/123/edit?bar, since they all identify the same resource.

[SergioBenitez]

That's not the right way to implement that functionality. The right thing to do would be to submit a PUT request with the button:

<form action="/foo/{{foo.id}}" method="post">
   <input type="hidden" name="_method" value="put" />
   <button type="submit">Edit</button>
</form>

Whether they're the same "resource" or not depends on your application; as far as I know, nothing defines whether a query string can cause a path to refer to a different resource or not. It's up to you to decide, and Rocket lets you do either.

It seems very odd that Rocket would route /foo/123/edit?anything=here to /foo/<id>/edit. If the client was expecting a query string, why didn't they match for it?

[lilyball]

My point is, if the server doesn't care about the query string, then it should just be ignoring it, instead of having the presence of a query string cause routing to fail. Again, I cannot think of any website that behaves the way Rocket is behaving. Any random website I can think of, slapping a query string on the / path returns me the index as if I didn't have a query.

[SergioBenitez]

"Any random website" doing something isn't a good reason for Rocket to do something. One of Rocket's main goals is to be as transparent as possible; it should be pretty obvious which handler is going to run given a request. I feel that this may muddy those waters.

Rocket doesn't just ignore things. It requires validation of every aspect of the request for processing. I believe it may go against Rocket's philosophy to silently toss away data. If you need a query string, you can match against the query string. If you don't care about it, don't match on it; only invalid requests will use a query strings. If there are two cases, you can use two different routes.

[lilyball]

Ok. I don't feel strongly enough about this to keep pushing for it, and you do appear to feel strongly about keeping the current behavior.

[SergioBenitez]

I do feel somewhat strongly about it, but that doesn't mean it's the right thing to do. ;) If there is some compelling reason why the current behavior is suboptimal as far as clarity, correctness, and safety goes, I'm happy to listen. (By the way, Rocket v0.1 actually did what you're proposing here, but it was changed for exactly the reasons I suggest above. I and others found it to be pretty confusing.)

In any case, I certainly hear where you're coming from, and I'll keep this in mind.

[lilyball]

I do think the case for treating an empty query string as if it's not there is stronger than the case for treating an arbitrary query string as if it's not there, because I think most people would consider http://example.com/foo? and http://example.com/foo to be the same URL.

[Eijebong]

For my part, I think that unless there is a route that actually cares about the query string, then rocket should ignore it.

If there is a route /foo, a route /foo?bleh=....
I should still be able to got to /foo?coucou=bar and be routed to /foo.

[SergioBenitez]

I thought about this issue some more. In the version of Rocket in master, what you've proposed works as you hoped. The full details of the changes can be seen in https://github.com/SergioBenitez/Rocket/commit/9160483554ef49586802bab66e112598efd8a509.

[sbwtw]

@SergioBenitez could you please release a new tag to include this change ?

[SergioBenitez]

@sbwtw Unfortunately, 9160483 is a breaking change, and so I'd need to release a new semver major version (0.3) to include it in a release. There are still a few outstanding issues that need to be resolved for 0.3. When those are ready, which I don't expect to take too much longer, 0.3 will ship with 9160483.