Skip to content

Commit b303f13

Browse files
committed
Merge branch 'feature/InvalidKeyException-NotFound' into develop
Previously, the `TopicController` would throw a 404 Not Found exception if a user requested e.g. `/Web/Invalid/Url`, but would throw a 500 Unhandled Exception if a user requested e.g. `/Web/Invalid/~Url`. That's because, under the hood, the `[ValidateTopic]` attribute calls into the `ITopicRepository.Load(RouteData)` extension method which, in turn, calls into the `TopicCollection<T>.GetTopic()` method, which will return an `InvalidKeyException` if the topic key passed doesn't confirm to OnTopic's standards. That's a useful default to help validate topic keys. But when we're dealing with topics passed by end users, we should expect bogus keys—and treat them no differently than any other invalid resource request. I.e., we should return a 404, not a 500. This is important as, currently, log files end up with a lot of false positives due to completely predictable 500 errors. Further, isolating common 404s is more difficult since they are distributed between 404s and 500s. To mitigate that, the `InvalidKeyException` is now swallowed as part of the `ITopicRepository.Load(RouteData)` extension method. Since that method is the primary point of looking up topics within the topic graph based on user-submitted route data, it not only addresses the `[ValidateTopic]` scenario, but also other potential cases where `RouteData` is being used to determine a topic. For instance, there are specialized cases where the `RouteData` is used to determine the `ContentTypeDescriptor` in the `IControllerActivator`, so this will address that scenario as well.
2 parents 0edab34 + 3211894 commit b303f13

File tree

1 file changed

+8
-1
lines changed

1 file changed

+8
-1
lines changed

OnTopic.AspNetCore.Mvc/TopicRepositoryExtensions.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,14 @@ RouteData routeData
7979
foreach (var searchPath in paths) {
8080
if (topic is not null) break;
8181
if (String.IsNullOrEmpty(searchPath)) continue;
82-
topic = topicRepository.Load(searchPath);
82+
try {
83+
topic = topicRepository.Load(searchPath);
84+
}
85+
catch (InvalidKeyException) {
86+
//As route data comes from user-submitted requests, it's expected that some may contain invalid keys. From this
87+
//method's perspective, this is no different than having an invalid URL. As such, we should return a null result, not
88+
//bubble up an exception.
89+
}
8390
}
8491

8592
return topic;

0 commit comments

Comments
 (0)