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

split_target over string beginning with exactly two slashes #248

Open
cuihtlauac opened this issue Jan 27, 2023 · 3 comments
Open

split_target over string beginning with exactly two slashes #248

cuihtlauac opened this issue Jan 27, 2023 · 3 comments

Comments

@cuihtlauac
Copy link

cuihtlauac commented Jan 27, 2023

I believe the behaviour of split_target is wrong when it is passed a string which begins with exactly two slashes:

>utop-full -require digestif.c
utop # #require "dream";;
utop # Dream.split_target "aaa/bbb/ccc";;
- : string * string = ("aaa/bbb/ccc", "")
utop # Dream.split_target "/aaa/bbb/ccc";;
- : string * string = ("/aaa/bbb/ccc", "")
utop # Dream.split_target "//aaa/bbb/ccc";; (* Boom! *)
- : string * string = ("/bbb/ccc", "")
utop # Dream.split_target "///aaa/bbb/ccc";;
- : string * string = ("/aaa/bbb/ccc", "")
utop # Dream.split_target "////aaa/bbb/ccc";;
- : string * string = ("//aaa/bbb/ccc", "")
utop # Dream.split_target "/////aaa/bbb/ccc";;
- : string * string = ("///aaa/bbb/ccc", "")
cuihtlauac pushed a commit to ocaml/ocaml.org that referenced this issue Jan 27, 2023
Handle the request unchanged if the target seems
canonical, otherwise:

  * Turns repeated slashes into a single one
  * Drop the last slash
  * Redirect to the resulting target

Note: Dream.split_targer bugged?
When passed a string beginning with exactly two slashes,
split_target drops everything before the first slash.
See: aantron/dream#248
As a work around, three slashes are preprended to all targets
cuihtlauac pushed a commit to ocaml/ocaml.org that referenced this issue Jan 27, 2023
Handle the request unchanged if the target seems
canonical, otherwise:

  * Turns repeated slashes into a single one
  * Drop the last slash
  * Redirect to the resulting target

Note: Dream.split_targer bugged?
When passed a string beginning with exactly two slashes,
split_target drops everything before the first slash.
See: aantron/dream#248
As a work around, three slashes are preprended to all targets
@cuihtlauac
Copy link
Author

cuihtlauac commented Jan 27, 2023

cuihtlauac added a commit to ocaml/ocaml.org that referenced this issue Feb 20, 2023
* Drop extra slashes

Handle the request unchanged if the target seems
canonical, otherwise:

  * Turns repeated slashes into a single one
  * Drop the last slash
  * Redirect to the resulting target

Note: Dream.split_targer bugged?
When passed a string beginning with exactly two slashes,
split_target drops everything before the first slash.
See: aantron/dream#248
As a work around, three slashes are preprended to all targets

* Formatting

---------

Co-authored-by: Cuihtlauac ALVARADO <[email protected]>
@aantron aantron added this to the alpha6 milestone Apr 14, 2023
@aantron
Copy link
Owner

aantron commented Apr 15, 2023

It's probably related to

(* TODO A very questionable interpretation of // as leading a hostname when we
know this is a target. There seems to be no way to work around this using
the interface of the Uri library. *)
decode "//abc/def";

and the test's output

and may be an upstream issue indeed. I am looking into this now. Ultimately, it should be possible to work around it by reimplementing it, even if it turns out to be an issue in Uri.

@aantron
Copy link
Owner

aantron commented Apr 15, 2023

I've left a detailed comment about this in the issue you opened, mirage/ocaml-uri#167 (thanks for that). I suggest we wait to see if there is a fix for this in ocaml-uri soon. If not, we can work around it in Dream for the next release.

@aantron aantron removed this from the alpha6 milestone Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants