Route matching too greedy: parsed segments may contain segment seperators

[ChrisBrandhorst]

Suppose we have the route /containers/{containerId}/contents.

If we request /containers/4/something/contents, this route will be matched and containerId will get a value of 4/something.

This seems counter-intuitive. I would expect {containerId} to only match a single path segment and think the currently implemented greedy alternative is a far less occurring situation.

The documentation seems to suggest this is by design. Is this really the case? If it is, I am interested in the reasoning behind this (since it conflicts with all the other routing mechanism I know of).

My proposed fix is not to match any character in a segment, but any non-slash character. Change at DefaultPatternMatcher#ParameterizeSegment line 141:
@"(?<{0}>.+?)"
becomes
@"(?<{0}>[^\/]+?)"

If one then actually wants to match the segment seperator, (s)he can always use the original RegEx: ?<foo>.+?.

[chandu]

If the defined route is /containers/{containerId}/contents and the requested route is /containers/4/something/contents, I would expect a 404 as the requested route doesn't match definedroute (something segment is not in the defined route and something should be considered as a separate segment)

[ChrisBrandhorst]

I concur.

[thecodejunkie]

@ChrisBrandhorst @chandu and I whole heatedly dis-concur. The route parameters were, explicitly, designed to be greedy by nature in the DefaultRoutePatternMatcher. Though it's also quite flexible when you need to control the nitty gritty details of the route.

You could for example

• Use a regex parameter to restrict the greediness. Using (?<containerId>[^/]*) in your route /containers/(?<containerId>[^/]*)/contents would give you the exact behavior you are asking for
• Implement your own IRoutePatternMatcher and support what ever route matching syntax you would like. The interface is really simple. Once implemented you need to override the InternalConfiguration property of your Bootstrapper and return something like NancyInternalConfiguration.WithOverrides(x => x.RoutePatternMatcher = typeof(MyRoutePatternMatcher)) and Nancy will start using your route matcher. The sky's the limit on this one

[ChrisBrandhorst]

Indeed I've already used the first suggestion you make: using a custom regex parameter in my route. However, in my project this behaviour is required in many places, so I thank you for your pointers on how to implement a custom pattern matcher (although I personally don't like that I now will need to copy the entire DefaultRoutePatternMatcher to replace one character in it, but that has already been noted in some other issue).

Although I respect your opinion, I don't understand it :-)
Could you elaborate on why you explicitily designed it this way? My personal biggest concern with it is that I feel the use case I present is a far more occuring situation then actually needing greedy matching. Thanks for your comments.

[codeprogression]

Other frameworks use both parameters and splats (I think this is what @ChrisBrandhorst is referring to when he mentioned other routing mechanisms). A parameter could be defined as {containerid} and a splat as {*containerid}.

[thecodejunkie]

@ChrisBrandhorst @codeprogression this is something we've been talking about the last couple of days and there are some things that needs to be ironed out before we can make a definite decision. Any changes to this would probably

• a breaking change since it would make sense to make the default behavior non-greedy if we were to change it
• not something that would be changed until the 0.14 time frame because 0.13 is pretty much done

[ChrisBrandhorst]

@codeprogression Correct.
@thecodejunkie Those seem good reasons to think carefully about it. About the breaking change: I would suggest some configuration in the bootstrapper to enable the non-greedy behavior so it won't be a breaking change. Perhaps in the future the switch can be made.