Skip to content
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

Fix EndpointRouting Guid regex + tests #594

Merged
merged 5 commits into from
May 12, 2024
Merged

Conversation

64J0
Copy link
Member

@64J0 64J0 commented May 10, 2024

Description

With this PR, I'm fixing the EndpointRouting Guid regex (route constraint) and adding more automated tests to assert that it's working.

How to test

You can use this project example:

open System
open Microsoft.AspNetCore.Builder
open Microsoft.Extensions.DependencyInjection
open Microsoft.Extensions.Hosting
open Giraffe
open Giraffe.EndpointRouting

let endpoints =
    [ GET
          [ route "/" (text "Hello World")
            routef "/try-a-guid/%O" (fun (guid: Guid) -> text $"Success: {guid}") ] ]

let notFoundHandler = "Not Found" |> text |> RequestErrors.notFound

let configureApp (appBuilder: IApplicationBuilder) =
    appBuilder.UseRouting().UseGiraffe(endpoints).UseGiraffe(notFoundHandler)

let configureServices (services: IServiceCollection) =
    services.AddRouting().AddGiraffe() |> ignore

[<EntryPoint>]
let main args =
    let builder = WebApplication.CreateBuilder(args)
    configureServices builder.Services

    let app = builder.Build()

    configureApp app
    app.Run()

    0

And test with some inputs. For example:

# must not find
curl http://localhost:5000/try-a-guid/8b3557db-fa-c0c90785ec0b

# must work fine
curl http://localhost:5000/try-a-guid/8b3557db-fa-c0c90785ec
# ...

You can find more Guid ideas at the automated tests file.

Related issues

@64J0 64J0 self-assigned this May 10, 2024
@64J0 64J0 requested review from dbrattli and nojaf May 10, 2024 20:28
@64J0 64J0 marked this pull request as ready for review May 10, 2024 20:28
@@ -13,7 +13,7 @@ open Giraffe

module private RouteTemplateBuilder =
let private guidPattern =
"([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}})"
"^[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}}$"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last part |^[-_0-9A-Za-z]{{22}}$ matches for a 22 char GUID e.g 0000000000000000000000 but there´s no test for it. Is there a format for such a GUID?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's this ShortGuid format. I'll add a test for it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with this commit b57fac8

@64J0 64J0 force-pushed the fix-endpoint-routing-guid-regex branch from 9cb923e to 4f87e76 Compare May 11, 2024 12:58
dbrattli
dbrattli previously approved these changes May 11, 2024
Copy link
Contributor

@dbrattli dbrattli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

@64J0 64J0 merged commit 69c92e6 into master May 12, 2024
6 checks passed
@64J0 64J0 deleted the fix-endpoint-routing-guid-regex branch May 12, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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