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

Bug: uri_string() does not return subfolders in baseURL #7124

Closed
kenjis opened this issue Jan 15, 2023 · 7 comments · Fixed by #7135
Closed

Bug: uri_string() does not return subfolders in baseURL #7124

kenjis opened this issue Jan 15, 2023 · 7 comments · Fixed by #7135
Assignees
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@kenjis
Copy link
Member

kenjis commented Jan 15, 2023

PHP Version

8.1

CodeIgniter4 Version

4.3.1

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

macOS

Which server did you use?

apache

Database

n/a

What happened?

uri_string() should return the absolute path part of the current URL.
https://codeigniter4.github.io/CodeIgniter4/helpers/url_helper.html#uri_string

But it returns only the route.

Steps to Reproduce

app.baseURL = 'http://localhost:8888/ci431/public/'

Navigate to http://localhost:8888/ci431/public/home.

d(uri_string()); // "home"
d(uri_string(true)); // "home"

Expected Output

If the current user guide is correct:

d(uri_string()); // "/ci431/public/home"
d(uri_string(true)); // "home"

Anything else?

Related #7123
The parameter $relative was introduced in #3682 aa48ef0 v4.0.5

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 15, 2023
@kenjis kenjis changed the title Bug: uri_string() doen not contain subfolders Bug: uri_string() doen not return subfolders Jan 15, 2023
@kenjis
Copy link
Member Author

kenjis commented Jan 15, 2023

The documented behavior of uri_string():

app.baseURL = http://some-site.com/subfolder/

uri_string(); // "/subfolder/blog/comments/123"

is not the same as CI3's uri_string().

CI3:
http://localhost:8888/ci3113/index.php/welcome/index:

uri_string(); // "welcome/index"

@kenjis kenjis changed the title Bug: uri_string() doen not return subfolders Bug: uri_string() doen not return subfolders in baseURL Jan 15, 2023
@kenjis kenjis changed the title Bug: uri_string() doen not return subfolders in baseURL Bug: uri_string() does not return subfolders in baseURL Jan 15, 2023
@kenjis
Copy link
Member Author

kenjis commented Jan 15, 2023

@MGatner aa48ef0
function uri_string(bool $relative = false): string

Why is the default $relative = false?
CI3's uri_string() returns only relative values.

@MGatner
Copy link
Member

MGatner commented Jan 15, 2023

That commit was to maintain the current default behavior. I was unaware of CI3's function at the time - not a deliberate change from its behavior.

@kenjis
Copy link
Member Author

kenjis commented Jan 15, 2023

Thank you.

I checked the behavior in v4.0.5. It is the same as v4.3.1.

app.baseURL = 'http://localhost:8888/ci405/public/'

d(uri_string());
d(uri_string(true));

Navigate to http://localhost:8888/ci405/public/home.

The results:

uri_string() string (4) "home"
uri_string(...) string (4) "home"

@kenjis
Copy link
Member Author

kenjis commented Jan 15, 2023

Probably uri_string(false) hasn't worked all along.
Because IncommingRequest's constructor changes the path in the URI object to the relative path (route).

@codeigniter4/core-team Can I remove the parameter as a bug fix?
I can't imagine a use case.

@MGatner
Copy link
Member

MGatner commented Jan 16, 2023

Fine by me. I will push again for a separate class to handle internal URIs; I think we need it despite the complexity it adds.

@kenjis
Copy link
Member Author

kenjis commented Jan 17, 2023

I sent a PR #7135

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants