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

Uri.of_string does not actually invoke pct_decode #31

Closed
nojb opened this issue Dec 28, 2013 · 3 comments
Closed

Uri.of_string does not actually invoke pct_decode #31

nojb opened this issue Dec 28, 2013 · 3 comments

Comments

@nojb
Copy link

nojb commented Dec 28, 2013

Is this expected behaviour?

> let s = "udp%3A//tracker.openbittorrent.com%3A80";;
> let uri = Uri.of_string s;;
val uri : Uri.t = <abstr>
> Uri.scheme uri;;
- : string option = None
> let uri = Uri.of_string (Uri.pct_decode s);;
val uri : Uri.t = <abstr>
> Uri.scheme uri;;
- : string option = Some "udp"
@dsheets
Copy link
Member

dsheets commented Dec 28, 2013

This is expected behavior:

> let s = "udp%3A//tracker.openbittorrent.com%3A80/%3A?%3A#%3A";;
val s : string = "udp%3A//tracker.openbittorrent.com%3A80/%3A?%3A#%3A"
> let uri = Uri.of_string s;;
val uri : Uri.t = udp%3A//tracker.openbittorrent.com%3A80/%3A?:#:
>  Uri.path uri;;
- : string = "udp://tracker.openbittorrent.com:80/:"

This is because ":" is not a valid character in the path component of a URI. If you know you have a URI which has syntactic components percent-encoded, you should first percent decode it before parsing it with Uri.of_string.

If the URI was percent-encoded because it was being passed as a component of another URI, this percent decoding will happen automatically and you can do something like:

let uri = match Uri.(get_query_param (of_string request_uri)) "target" with
| None -> raise Not_found
| Some t -> Uri.of_string t

I think this behavior makes sense but I'm interested in hearing your opinion. Please note, there is also a rather nasty percent-encoding bug which I intend to fix soon (later today?).

@nojb
Copy link
Author

nojb commented Dec 28, 2013

Dear David,

Thanks very much for your reply! I am rather ignorant about the technicalities
of the URI format, but the comment in uri.mli is

(** Convert a percent-encoded string into a URI structure *)
val of_string : string -> t

which is confusing IMHO.

Thanks again!
Nicolas

On 28 Dec 2013, at 16:30, David Sheets [email protected] wrote:

This is expected behavior:

let s = "udp%3A//tracker.openbittorrent.com%3A80/%3A?%3A#%3A";;
val s : string = "udp%3A//tracker.openbittorrent.com%3A80/%3A?%3A#%3A"
let uri = Uri.of_string s;;
val uri : Uri.t = udp%3A//tracker.openbittorrent.com%3A80/%3A?:#:
Uri.path uri;;

  • : string = "udp://tracker.openbittorrent.com:80/:"
    This is because ":" is not a valid character in the path component of a URI. If you know you have a URI which has syntactic components percent-encoded, you should first percent decode it before parsing it with Uri.of_string.

If the URI was percent-encoded because it was being passed as a component of another URI, this percent encoding will happen automatically and you can do something like:

let uri = match Uri.get_query_param (of_string request_uri) "target" with
| None -> raise Not_found
| Some t -> Uri.of_string t
I think this behavior makes sense but I'm interested in hearing your opinion. Please note, there is also a rather nasty percent-encoding bug which I intend to fix soon (later today?).


Reply to this email directly or view it on GitHub.

@dsheets
Copy link
Member

dsheets commented Dec 28, 2013

Oh, I see. That is quite confusing. I will explain more clearly what of_string does in my patch to fix the above percent-encoding bug. It is quite subtle as the Uri.(to_string (of_string uri_str)) will actually perform something like normalization. Thanks for your report!

@avsm avsm closed this as completed in 0585f54 Dec 28, 2013
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

No branches or pull requests

2 participants