-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Guid pattern in endpoint router matches invalid values, throws System.FormatException #576
Comments
What's the behavior you're after? Why do you consider the above a bug? |
We'd rather it return 4xx, and IMO in general, user input should not result in unhandled exceptions. |
Hey, I did explore this problem today and would like to add some comments. First, I noticed that this problem is happening with Request reached the end of the middleware pipeline without being handled by application code.
Request path: GET http://localhost:5000/try-a-guid/00000000-0000-0000-0000-0000000000123,
Response status code: 404 I was using this code (Giraffe router): open System
open Microsoft.AspNetCore.Builder
open Microsoft.AspNetCore.Hosting
open Microsoft.Extensions.Hosting
open Microsoft.Extensions.Logging
open Microsoft.Extensions.DependencyInjection
open Giraffe
let webApp =
GET >=> routef "/try-a-guid/%O" (fun (guid: Guid) -> text $"Success: {guid}" )
type Startup() =
member __.ConfigureServices (services : IServiceCollection) =
// Register default Giraffe dependencies
services.AddGiraffe() |> ignore
member __.Configure (app : IApplicationBuilder)
(env : IHostEnvironment)
(loggerFactory : ILoggerFactory) =
// Add Giraffe to the ASP.NET Core pipeline
app.UseGiraffe webApp
[<EntryPoint>]
let main _ =
Host.CreateDefaultBuilder()
.ConfigureWebHostDefaults(
fun webHostBuilder ->
webHostBuilder
.UseStartup<Startup>()
|> ignore)
.Build()
.Run()
0 And I confirm that this problem is still happening in the most recent version of Giraffe. At first, I thought it was not a good idea to add some hidden feature that maps incorrect regex parsing to a generic error response. It could be a pain to debug (imagine having an API with 1_000_000s of requests, and out of nowhere some start returning an unexpected status code + response which you don't find in your codebase). Then, after digging deeper into the problem, I think it's better to handle it properly, at least trying to replicate what we have with Giraffe's router. It must demand simply a tweak in the GUID regex pattern. I'll take a look at it this week, and probably add some new tests. |
Feel free to add your review to this PR #594 |
Giraffe version |
Looks good to me. Thanks! |
Our fuzzer discovered some values that will match the endpoint router's regex behind
%O
.App code:
Test:
Full repro here.
One workaround is adding middleware to catch
FormatException
.dotnet sdk 8.0.200
Giraffe 6.2.0
The text was updated successfully, but these errors were encountered: