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

More robust handling of paths beginning with // #308

Closed
zoggy opened this issue Apr 9, 2015 · 6 comments
Closed

More robust handling of paths beginning with // #308

zoggy opened this issue Apr 9, 2015 · 6 comments

Comments

@zoggy
Copy link
Contributor

zoggy commented Apr 9, 2015

Hello,

When an uri such as http://localhost//foo/bar is requested, Uri.of_string is used on the requested path, i.e. //foo/bar, which returns an uri with path /bar, because it handles the whole string as a uri. This should be parsed as a path, to handle "incorrect" paths beginning with "//".

The call to Uri.of_string is here: https://github.com/mirage/ocaml-cohttp/blob/master/lib/request.ml#L113

@dsheets
Copy link
Member

dsheets commented Apr 9, 2015

Indeed you are correct. I am preparing a patch now.

@dsheets
Copy link
Member

dsheets commented Apr 9, 2015

#309 fixes this issue. When that PR is merged, this issue will close. Thanks for reporting the problem!

@dsheets dsheets closed this as completed in 3f09e5d Apr 9, 2015
@zoggy
Copy link
Contributor Author

zoggy commented Apr 14, 2015

The fix does not seem to handle query params correctly, i.e. with the following url:

http://localhost/test?formula=foo

the path obtained with Uri.path is /test%3Fformula=foo instead of "/test".

@dsheets
Copy link
Member

dsheets commented Apr 14, 2015

Indeed. :-( Sorry about that. It's the same problem as mirage/irmin#187 (comment).

@dsheets dsheets reopened this Apr 14, 2015
@zoggy
Copy link
Contributor Author

zoggy commented Apr 14, 2015

I'm trying to fix it.

@zoggy
Copy link
Contributor Author

zoggy commented Apr 14, 2015

Here is a PR: #317
"make test" was OK and it seems to fix my problem.

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