Skip to content

Commit 9c0141a

Browse files
kenjiskenjis
authored
Merge pull request #7215 from kenjis/rework-URI-related-code
fix: site_url() does not use alt Config and refactoring
2 parents 0a4209e + b9cae3c commit 9c0141a

7 files changed

Lines changed: 160 additions & 39 deletions

File tree

‎system/HTTP/IncomingRequest.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -443,14 +443,17 @@ public function isSecure(): bool
443443
* instance, this can be used to change the "current URL"
444444
* for testing.
445445
*
446-
* @param string $path URI path relative to SCRIPT_NAME
446+
* @param string $path URI path relative to baseURL
447447
* @param App|null $config Optional alternate config to use
448448
*
449449
* @return $this
450450
*/
451451
public function setPath(string $path, ?App $config = null)
452452
{
453453
$this->path = $path;
454+
455+
// @TODO remove this. The path of the URI object should be a full URI path,
456+
// not a URI path relative to baseURL.
454457
$this->uri->setPath($path);
455458

456459
$config ??= $this->config;
@@ -481,8 +484,11 @@ public function setPath(string $path, ?App $config = null)
481484
$this->uri->setScheme('https');
482485
}
483486
} elseif (! is_cli()) {
487+
// Do not change exit() to exception; Request is initialized before
488+
// setting the exception handler, so if an exception is raised, an
489+
// error will be displayed even if in the production environment.
484490
// @codeCoverageIgnoreStart
485-
exit('You have an empty or invalid base URL. The baseURL value must be set in Config\App.php, or through the .env file.');
491+
exit('You have an empty or invalid baseURL. The baseURL value must be set in app/Config/App.php, or through the .env file.');
486492
// @codeCoverageIgnoreEnd
487493
}
488494

‎system/HTTP/URI.php‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,9 @@ class URI
148148
/**
149149
* Builds a representation of the string from the component parts.
150150
*
151-
* @param string|null $scheme URI scheme. E.g., http, ftp
152-
* @param string $authority
153-
* @param string $path
154-
* @param string $query
155-
* @param string $fragment
151+
* @param string|null $scheme URI scheme. E.g., http, ftp
152+
*
153+
* @return string URI string with only passed parts. Maybe incomplete as a URI.
156154
*/
157155
public static function createURIString(
158156
?string $scheme = null,

‎system/Helpers/url_helper.php‎

Lines changed: 55 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,34 @@
2626
*
2727
* @internal Outside the framework this should not be used directly.
2828
*
29-
* @param string $relativePath May include queries or fragments
29+
* @param array|string $relativePath URI string or array of URI segments.
30+
* May include queries or fragments.
31+
* @param App|null $config Alternative Config to use
3032
*
3133
* @throws HTTPException For invalid paths.
3234
* @throws InvalidArgumentException For invalid config.
3335
*/
34-
function _get_uri(string $relativePath = '', ?App $config = null): URI
36+
function _get_uri($relativePath = '', ?App $config = null): URI
3537
{
36-
$config ??= config('App');
38+
$appConfig = null;
39+
if ($config === null) {
40+
/** @var App $appConfig */
41+
$appConfig = config('App');
42+
43+
if ($appConfig->baseURL === '') {
44+
throw new InvalidArgumentException(
45+
'_get_uri() requires a valid baseURL.'
46+
);
47+
}
48+
} elseif ($config->baseURL === '') {
49+
throw new InvalidArgumentException(
50+
'_get_uri() requires a valid baseURL.'
51+
);
52+
}
3753

38-
if ($config->baseURL === '') {
39-
throw new InvalidArgumentException('_get_uri() requires a valid baseURL.');
54+
// Convert array of segments to a string
55+
if (is_array($relativePath)) {
56+
$relativePath = implode('/', $relativePath);
4057
}
4158

4259
// If a full URI was passed then convert it
@@ -53,27 +70,31 @@ function _get_uri(string $relativePath = '', ?App $config = null): URI
5370

5471
$relativePath = URI::removeDotSegments($relativePath);
5572

56-
// Build the full URL based on $config and $relativePath
5773
$request = Services::request();
5874

59-
/** @var App $config */
60-
$url = $request instanceof CLIRequest
61-
? rtrim($config->baseURL, '/ ') . '/'
62-
: $request->getUri()->getBaseURL();
75+
if ($config === null) {
76+
$baseURL = $request instanceof CLIRequest
77+
? rtrim($appConfig->baseURL, '/ ') . '/'
78+
// Use the current baseURL for multiple domain support
79+
: $request->getUri()->getBaseURL();
80+
81+
$config = $appConfig;
82+
} else {
83+
$baseURL = rtrim($config->baseURL, '/ ') . '/';
84+
}
6385

6486
// Check for an index page
87+
$indexPage = '';
6588
if ($config->indexPage !== '') {
66-
$url .= $config->indexPage;
89+
$indexPage = $config->indexPage;
6790

6891
// Check if we need a separator
6992
if ($relativePath !== '' && $relativePath[0] !== '/' && $relativePath[0] !== '?') {
70-
$url .= '/';
93+
$indexPage .= '/';
7194
}
7295
}
7396

74-
$url .= $relativePath;
75-
76-
$uri = new URI($url);
97+
$uri = new URI($baseURL . $indexPage . $relativePath);
7798

7899
// Check if the baseURL scheme needs to be coerced into its secure version
79100
if ($config->forceGlobalSecureRequests && $uri->getScheme() === 'http') {
@@ -94,11 +115,6 @@ function _get_uri(string $relativePath = '', ?App $config = null): URI
94115
*/
95116
function site_url($relativePath = '', ?string $scheme = null, ?App $config = null): string
96117
{
97-
// Convert array of segments to a string
98-
if (is_array($relativePath)) {
99-
$relativePath = implode('/', $relativePath);
100-
}
101-
102118
$uri = _get_uri($relativePath, $config);
103119

104120
return URI::createURIString(
@@ -121,7 +137,15 @@ function site_url($relativePath = '', ?string $scheme = null, ?App $config = nul
121137
*/
122138
function base_url($relativePath = '', ?string $scheme = null): string
123139
{
124-
$config = clone config('App');
140+
/** @var App $config */
141+
$config = clone config('App');
142+
143+
// Use the current baseURL for multiple domain support
144+
$request = Services::request();
145+
$config->baseURL = $request instanceof CLIRequest
146+
? rtrim($config->baseURL, '/ ') . '/'
147+
: $request->getUri()->getBaseURL();
148+
125149
$config->indexPage = '';
126150

127151
return site_url($relativePath, $scheme, $config);
@@ -131,27 +155,29 @@ function base_url($relativePath = '', ?string $scheme = null): string
131155
if (! function_exists('current_url')) {
132156
/**
133157
* Returns the current full URL based on the Config\App settings and IncomingRequest.
134-
* String returns ignore query and fragment parts.
135158
*
136159
* @param bool $returnObject True to return an object instead of a string
137160
* @param IncomingRequest|null $request A request to use when retrieving the path
138161
*
139-
* @return string|URI
162+
* @return string|URI When returning string, the query and fragment parts are removed.
163+
* When returning URI, the query and fragment parts are preserved.
140164
*/
141165
function current_url(bool $returnObject = false, ?IncomingRequest $request = null)
142166
{
143167
$request ??= Services::request();
144-
$path = $request->getPath();
168+
/** @var CLIRequest|IncomingRequest $request */
169+
$routePath = $request->getPath();
170+
$currentURI = $request->getUri();
145171

146172
// Append queries and fragments
147-
if ($query = $request->getUri()->getQuery()) {
148-
$path .= '?' . $query;
173+
if ($query = $currentURI->getQuery()) {
174+
$query = '?' . $query;
149175
}
150-
if ($fragment = $request->getUri()->getFragment()) {
151-
$path .= '#' . $fragment;
176+
if ($fragment = $currentURI->getFragment()) {
177+
$fragment = '#' . $fragment;
152178
}
153179

154-
$uri = _get_uri($path);
180+
$uri = _get_uri($routePath . $query . $fragment);
155181

156182
return $returnObject ? $uri : URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath());
157183
}

‎tests/system/HTTP/URITest.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,22 @@ public function testSetURISilent()
978978
$this->assertTrue(true);
979979
}
980980

981+
public function testCreateURIStringNoArguments()
982+
{
983+
$uri = URI::createURIString();
984+
985+
$expected = '';
986+
$this->assertSame($expected, $uri);
987+
}
988+
989+
public function testCreateURIStringOnlyAuthority()
990+
{
991+
$uri = URI::createURIString(null, 'example.com');
992+
993+
$expected = 'example.com';
994+
$this->assertSame($expected, $uri);
995+
}
996+
981997
public function testCreateURIString()
982998
{
983999
$uri = URI::createURIString('https', 'example.com', '/');

‎tests/system/Helpers/URLHelper/CurrentUrlTest.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ public function testCurrentURLReturnsBasicURL()
6363

6464
$this->config->baseURL = 'http://example.com/public';
6565

66+
// URI object are updated in IncomingRequest constructor.
67+
$request = Services::incomingrequest($this->config);
68+
Services::injectMock('request', $request);
69+
6670
$this->assertSame('http://example.com/public/index.php/', current_url());
6771
}
6872

‎tests/system/Helpers/URLHelper/SiteUrlTest.php‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,33 @@ public function configProvider()
139139
'http://example.com/abc',
140140
'http://example.com/abc',
141141
],
142+
[
143+
'http://example.com/',
144+
'',
145+
null,
146+
false,
147+
'/abc',
148+
'http://example.com/abc',
149+
'http://example.com/abc',
150+
],
151+
[
152+
'http://example.com/',
153+
'',
154+
null,
155+
false,
156+
'/abc/',
157+
'http://example.com/abc/',
158+
'http://example.com/abc/',
159+
],
160+
[
161+
'http://example.com/',
162+
'',
163+
null,
164+
false,
165+
'/abc/def',
166+
'http://example.com/abc/def',
167+
'http://example.com/abc/def',
168+
],
142169
'URL decode' => [
143170
'http://example.com/',
144171
'',
@@ -314,6 +341,24 @@ public function testBaseURLService()
314341
$this->assertSame('http://example.com/ci/v4/controller/method', base_url('controller/method', null));
315342
}
316343

344+
public function testBaseURLWithCLIRequest()
345+
{
346+
unset($_SERVER['HTTP_HOST'], $_SERVER['REQUEST_URI']);
347+
348+
$this->config->baseURL = 'http://example.com/';
349+
$request = Services::clirequest($this->config);
350+
Services::injectMock('request', $request);
351+
352+
$this->assertSame(
353+
'http://example.com/index.php/controller/method',
354+
site_url('controller/method', null, $this->config)
355+
);
356+
$this->assertSame(
357+
'http://example.com/controller/method',
358+
base_url('controller/method', null)
359+
);
360+
}
361+
317362
public function testSiteURLWithAllowedHostname()
318363
{
319364
$_SERVER['HTTP_HOST'] = 'www.example.jp';
@@ -322,14 +367,37 @@ public function testSiteURLWithAllowedHostname()
322367

323368
$this->config->baseURL = 'http://example.com/public/';
324369
$this->config->allowedHostnames = ['www.example.jp'];
370+
Services::injectMock('config', $this->config);
325371

326372
// URI object are updated in IncomingRequest constructor.
327373
$request = Services::incomingrequest($this->config);
328374
Services::injectMock('request', $request);
329375

330376
$this->assertSame(
331377
'http://www.example.jp/public/index.php/controller/method',
332-
site_url('controller/method', null, $this->config)
378+
site_url('controller/method')
379+
);
380+
}
381+
382+
public function testSiteURLWithAltConfig()
383+
{
384+
$_SERVER['HTTP_HOST'] = 'www.example.jp';
385+
$_SERVER['REQUEST_URI'] = '/public';
386+
$_SERVER['SCRIPT_NAME'] = '/public/index.php';
387+
388+
$this->config->baseURL = 'http://example.com/public/';
389+
$this->config->allowedHostnames = ['www.example.jp'];
390+
391+
// URI object are updated in IncomingRequest constructor.
392+
$request = Services::incomingrequest($this->config);
393+
Services::injectMock('request', $request);
394+
395+
$altConfig = clone $this->config;
396+
$altConfig->baseURL = 'http://alt.example.com/public/';
397+
398+
$this->assertSame(
399+
'http://alt.example.com/public/index.php/controller/method',
400+
site_url('controller/method', null, $altConfig)
333401
);
334402
}
335403

‎user_guide_src/source/helpers/url_helper.rst‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,17 +98,20 @@ The following functions are available:
9898
:returns: The current URL
9999
:rtype: string|\\CodeIgniter\\HTTP\\URI
100100

101-
Returns the full URL (including segments) of the page being currently viewed.
101+
Returns the full URL of the page being currently viewed.
102+
When returning string, the query and fragment parts of the URL are removed.
103+
When returning URI, the query and fragment parts are preserved.
102104

103105
However for security reasons, it is created based on the ``Config\App`` settings,
104106
and not intended to match the browser URL.
105107

106108
Since v4.3.0, if you set ``Config\App::$allowedHostnames``,
107109
this returns the URL with the hostname set in it if the current URL matches.
108110

109-
.. note:: Calling this function is the same as doing this:
111+
.. note:: Calling ``current_url()`` is the same as doing this:
110112

111113
.. literalinclude:: url_helper/006.php
114+
:lines: 2-
112115

113116
.. important:: Prior to v4.1.2, this function had a bug causing it to ignore the configuration on ``Config\App::$indexPage``.
114117

0 commit comments

Comments
 (0)