Guid pattern in endpoint router matches invalid values, throws System.FormatException

[jcmrva]

Our fuzzer discovered some values that will match the endpoint router's regex behind %O.

App code:

GET [ routef "/try-a-guid/%O" (fun (guid: Guid) -> text $"Success: {guid}" ) ]

Test:

#r "nuget: FsHttp"
open FsHttp

let fuzzerInput = "8b3557db-fa-c0c90785ec0b"
http {
    GET $"http://localhost:5000/try-a-guid/{fuzzerInput}"
}
|> Request.send

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/try-a-guid/8b3557db-fa-c0c90785ec0b - - -
info: Microsoft.AspNetCore.Routing.EndpointMiddleware[0]
      Executing endpoint 'HTTP: GET /try-a-guid/{O0:regex(([0-9A-Fa-f]{{8}}\-[0-9A-Fa-f]{{4}}\-[0-9A-Fa-f]{{4}}\-[0-9A-Fa-f]{{4}}\-[0-9A-Fa-f]{{12}}|[0-9A-Fa-f]{{32}}|[-_0-9A-Za-z]{{22}}))}'
info: Microsoft.AspNetCore.Routing.EndpointMiddleware[1]
      Executed endpoint 'HTTP: GET /try-a-guid/{O0:regex(([0-9A-Fa-f]{{8}}\-[0-9A-Fa-f]{{4}}\-[0-9A-Fa-f]{{4}}\-[0-9A-Fa-f]{{4}}\-[0-9A-Fa-f]{{12}}|[0-9A-Fa-f]{{32}}|[-_0-9A-Za-z]{{22}}))}'
fail: Microsoft.AspNetCore.Server.Kestrel[13]
      Connection id "0HN1J1DK6DFOF", Request id "0HN1J1DK6DFOF:00000001": An unhandled exception was thrown by the application.
      System.FormatException: Unrecognized Guid format.
         at System.Guid.GuidResult.SetFailure(ParseFailure failureKind)
         at System.Guid.TryParseGuid(ReadOnlySpan`1 guidString, GuidResult& result)
         at System.Guid..ctor(String g)
         at Giraffe.EndpointRouting.RequestDelegateBuilder.parseGuid@56-1(String s)
         at Giraffe.EndpointRouting.RequestDelegateBuilder.tryGetParser@68-6.Invoke(String x)
         at Giraffe.EndpointRouting.RequestDelegateBuilder.convertToTuple(FSharpList`1 mappings, RouteData routeData)
         at Giraffe.EndpointRouting.RequestDelegateBuilder.createTokenizedRequestDelegate@111-1.MoveNext()
         at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|7_0(Endpoint endpoint, Task requestTask, ILogger logger)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished HTTP/1.1 GET http://localhost:5000/try-a-guid/8b3557db-fa-c0c90785ec0b - 500 0 - 14.3764ms

Full repro here.

One workaround is adding middleware to catch FormatException.

dotnet sdk 8.0.200 Giraffe 6.2.0

[ronnieholm]

What's the behavior you're after? Why do you consider the above a bug?

[jcmrva]

We'd rather it return 4xx, and IMO in general, user input should not result in unhandled exceptions.

[64J0]

Hey, I did explore this problem today and would like to add some comments.

First, I noticed that this problem is happening with Giraffe.EndpointRouting. I did test with Giraffe router, and it simply ignores the request, going to the response status 404:

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). ~If you're using this %O pattern, it's your responsibility to grant that the input will be GUID-compliant. Otherwise, if you want to avoid this problem, I would recommend using %s and then, inside the endpoint handler, you try to convert to GUID.~

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.

[64J0]

Feel free to add your review to this PR https://github.com/giraffe-fsharp/Giraffe/pull/594

[64J0]

Giraffe version 6.4.1-alpha-3 has the fix. I tested locally, and it's not matching the wrong Guid. Please let me know if there's something else regarding this issue @jcmrva.

[jcmrva]

Looks good to me. Thanks!