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

URL::site() should always rawurlencode uri #631

Open
kohamkohane opened this issue Aug 4, 2015 · 7 comments
Open

URL::site() should always rawurlencode uri #631

kohamkohane opened this issue Aug 4, 2015 · 7 comments

Comments

@kohamkohane
Copy link

Now URL::site() use rawurlencode only if UTF8::is_ascii($path) is FALSE. Why? I suggest always encode uri. For example space is ascii and should be encoded!

@enov
Copy link
Contributor

enov commented Aug 11, 2015

Yeah, I think you are right on this one. I did not do any tests, but even those URL delimiters ? and & are ASCII. Could you provide a PR to fix with unit tests (if applicable)?

@kohamkohane
Copy link
Author

I could, but first let's think how rawurlencode should works in kohana. If we add rawurlencode to URL:site (in my opinion the best choicie) then we should not rawurlencode in route->url() becouse it use URL:site() so we double rawurlencode then. I think we should only rawurlencode in URL::site() no in route. What do you think about it?

By the way, does route->uri() (URI not URL) should be rawurlencoded too?

@kohamkohane
Copy link
Author

Or maybe different view. Rawurlencode parameters in route->uri() as now, and do not rawurlencode in URL:site() or rawurlencode only if URI passed to URL:site() was not encoded earilier.

Anyway current if(UTF8::is_ascii($path)) in URL::site is wrong.

@enov
Copy link
Contributor

enov commented Aug 13, 2015

I am leaning towards removing rawurlencode from URL::site as it is assuming that a forward slash should always be treated as URL parameter separator.

There is no way to encode a slash in there, even if the slash itself is a sub-string of a parameter. URL::site, unlike Route::uri, does not have prior knowledge of parameters.

But that means breaking existing applications, so it should be done in v3.4.

Let me know what do you think.

@enov
Copy link
Contributor

enov commented Aug 13, 2015

cc @acoulton

@kohamkohane
Copy link
Author

I agree with you. Just remove rawurlencode from URL::site.

@acoulton
Copy link
Member

@enov I think I agree - remove rawurlencode from URL::site

@neo22s neo22s added this to the 4.0.0 milestone Mar 21, 2016
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

4 participants