andreas / ocaml-graphql-server

GraphQL servers in OCaml
MIT License
624 stars 60 forks source link

Fix routing in graphql-cohttp #150

Closed andreas closed 5 years ago

andreas commented 5 years ago

146 inadvertently introduced a bug due to the different handling of empty parts when splitting strings:

(* Old implementation *)
Str.(split (regexp "/") "/graphql"
- : ["graphql"] : string list

(* New implementation *)
Astring.String.cuts ~sep:"/" "/graphql"
- : [""; "graphql"]

Astring.String.cuts ~sep:"/" "/graphql/"
- : [""; "graphql"; ""]

By adding ~empty:false, the old behavior can be restored:

Astring.String.cuts ~empty:false ~sep:"/" "/graphql"
- : ["graphql"]

Astring.String.cuts ~empty:false ~sep:"/" "/graphql/"
- : ["graphql"]

This is preferable over changing the pattern matching (#149) due to handling of the trailing slashes.

phated commented 5 years ago

@andreas and @anmonteiro thanks! I was smashing my head against this for so long before I reached out. Glad this is getting a fix 😄

andreas commented 5 years ago

@phated Sorry about that 😢