Skip to content

Commit 3d0151c

Browse files
emmadesilvaemmadesilva
authored
Merge pull request #1726 from hydephp/ensure-a-better-state-when-a-site-url-is-not-set
Improve behaviour when a site URL is not explicitly set
2 parents 6efd034 + 0bcb9f7 commit 3d0151c

File tree

8 files changed

+280
-22
lines changed

8 files changed

+280
-22
lines changed

‎RELEASE_NOTES.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ This serves two purposes:
1818
### Changed
1919
- When a navigation group is set in front matter, it will now be used regardless of the subdirectory configuration in https://github.com/hydephp/develop/pull/1703 (fixes https://github.com/hydephp/develop/issues/1515)
2020
- Use late static bindings to support overriding data collections file finding in https://github.com/hydephp/develop/pull/1717 (fixes https://github.com/hydephp/develop/issues/1716)
21+
- Method `Hyde::hasSiteUrl()` now returns false if the site URL is for localhost in https://github.com/hydephp/develop/pull/1726
22+
- Method `Hyde::url()` will now return a relative URL instead of throwing an exception when supplied a path even if the site URL is not set in https://github.com/hydephp/develop/pull/1726
2123

2224
### Deprecated
2325
- for soon-to-be removed features.
@@ -32,3 +34,17 @@ This serves two purposes:
3234

3335
### Security
3436
- in case of vulnerabilities.
37+
38+
### Extra information
39+
40+
This release contains changes to how HydePHP behaves when a site URL is not set by the user.
41+
42+
These changes are made to reduce the chance of the default `localhost` value showing up in production environments.
43+
44+
Most notably, HydePHP now considers that default site URL `localhost` to mean that a site URL is not set, as the user has not set it.
45+
This means that things like automatic canonical URLs will not be added, as Hyde won't know how to make them without a site URL.
46+
The previous behaviour was that Hyde used `localhost` in canonical URLs, which is never useful in production environments.
47+
48+
For this reason, we felt it worth it to make this change in a minor release, as it has a such large benefit for sites.
49+
50+
You can read more about the details and design decisions of this change in the following pull request https://github.com/hydephp/develop/pull/1726.

‎packages/framework/src/Foundation/Kernel/Hyperlinks.php‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,14 @@ public function asset(string $name, bool $preferQualifiedUrl = false): string
126126

127127
/**
128128
* Check if a site base URL has been set in config (or .env).
129+
*
130+
* The default value is `http://localhost`, which is not considered a valid site URL.
129131
*/
130132
public function hasSiteUrl(): bool
131133
{
132-
return ! blank(Config::getNullableString('hyde.url'));
134+
$value = Config::getNullableString('hyde.url');
135+
136+
return ! blank($value) && $value !== 'http://localhost';
133137
}
134138

135139
/**
@@ -147,6 +151,13 @@ public function url(string $path = ''): string
147151
return rtrim(rtrim(Config::getString('hyde.url'), '/')."/$path", '/');
148152
}
149153

154+
// Since v1.7.0, we return the relative path even if the base URL is not set,
155+
// as this is more likely to be the desired behavior the user's expecting.
156+
if (! blank($path)) {
157+
return $path;
158+
}
159+
160+
// User is trying to get the base URL, but it's not set
150161
throw new BaseUrlNotSetException();
151162
}
152163

‎packages/framework/src/Framework/Actions/PostBuildTasks/GenerateSitemap.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use Hyde\Framework\Concerns\InteractsWithDirectories;
1010
use Hyde\Framework\Features\XmlGenerators\SitemapGenerator;
1111

12-
use function blank;
1312
use function file_put_contents;
1413

1514
class GenerateSitemap extends PostBuildTask
@@ -22,7 +21,7 @@ class GenerateSitemap extends PostBuildTask
2221

2322
public function handle(): void
2423
{
25-
if (blank(Hyde::url()) || str_starts_with(Hyde::url(), 'http://localhost')) {
24+
if (! Hyde::hasSiteUrl()) {
2625
$this->skip('Cannot generate sitemap without a valid base URL');
2726
}
2827

‎packages/framework/tests/Feature/Foundation/HyperlinksTest.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ public function testAssetHelperResolvesPathsForNestedPages()
5656

5757
public function testAssetHelperReturnsQualifiedAbsoluteUriWhenRequestedAndSiteHasBaseUrl()
5858
{
59-
$this->assertSame('http://localhost/media/test.jpg', $this->class->asset('test.jpg', true));
59+
config(['hyde.url' => 'https://example.org']);
60+
$this->assertSame('https://example.org/media/test.jpg', $this->class->asset('test.jpg', true));
6061
}
6162

6263
public function testAssetHelperReturnsDefaultRelativePathWhenQualifiedAbsoluteUriIsRequestedButSiteHasNoBaseUrl()
@@ -65,11 +66,22 @@ public function testAssetHelperReturnsDefaultRelativePathWhenQualifiedAbsoluteUr
6566
$this->assertSame('media/test.jpg', $this->class->asset('test.jpg', true));
6667
}
6768

69+
public function testAssetHelperReturnsDefaultRelativePathWhenQualifiedAbsoluteUriIsRequestedButSiteBaseUrlIsLocalhost()
70+
{
71+
$this->assertSame('media/test.jpg', $this->class->asset('test.jpg', true));
72+
}
73+
6874
public function testAssetHelperReturnsInputWhenQualifiedAbsoluteUriIsRequestedButImageIsAlreadyQualified()
6975
{
7076
$this->assertSame('http://localhost/media/test.jpg', $this->class->asset('http://localhost/media/test.jpg', true));
7177
}
7278

79+
public function testAssetHelperReturnsInputWhenQualifiedAbsoluteUriIsRequestedButImageIsAlreadyQualifiedRegardlessOfMatchingTheConfiguredUrl()
80+
{
81+
config(['hyde.url' => 'https://example.org']);
82+
$this->assertSame('http://localhost/media/test.jpg', $this->class->asset('http://localhost/media/test.jpg', true));
83+
}
84+
7385
public function testAssetHelperUsesConfiguredMediaDirectory()
7486
{
7587
Hyde::setMediaDirectory('_assets');

‎packages/framework/tests/Feature/HelpersTest.php‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,14 @@ public function testAssetFunction()
7777
public function testAssetFunctionWithQualifiedUrl()
7878
{
7979
$this->assertSame(Hyde::asset('foo', true), asset('foo', true));
80-
$this->assertSame('http://localhost/media/foo', asset('foo', true));
80+
$this->assertSame('media/foo', asset('foo', true));
81+
}
82+
83+
/** @covers ::asset */
84+
public function testAssetFunctionWithQualifiedUrlAndSetBaseUrl()
85+
{
86+
$this->app['config']->set(['hyde.url' => 'https://example.com']);
87+
$this->assertSame('https://example.com/media/foo', asset('foo', true));
8188
}
8289

8390
/** @covers ::asset */
@@ -94,6 +101,13 @@ public function testAssetFunctionWithQualifiedUrlAndNoBaseUrl()
94101
$this->assertSame('media/foo', asset('foo', true));
95102
}
96103

104+
/** @covers ::asset */
105+
public function testAssetFunctionWithQualifiedUrlAndLocalhostBaseUrl()
106+
{
107+
$this->app['config']->set(['hyde.url' => 'http://localhost']);
108+
$this->assertSame('media/foo', asset('foo', true));
109+
}
110+
97111
/** @covers ::asset */
98112
public function testAssetFunctionFromNestedPage()
99113
{
@@ -145,17 +159,39 @@ public function testUrlFunction()
145159

146160
/** @covers ::url */
147161
public function testUrlFunctionWithBaseUrl()
162+
{
163+
$this->app['config']->set(['hyde.url' => 'https://example.com']);
164+
$this->assertSame('https://example.com/foo', url('foo'));
165+
}
166+
167+
/** @covers ::url */
168+
public function testUrlFunctionWithLocalhostBaseUrl()
148169
{
149170
$this->app['config']->set(['hyde.url' => 'http://localhost']);
150-
$this->assertSame('http://localhost/foo', url('foo'));
171+
$this->assertSame('foo', url('foo'));
151172
}
152173

153174
/** @covers ::url */
154175
public function testUrlFunctionWithoutBaseUrl()
155176
{
156177
$this->app['config']->set(['hyde.url' => null]);
178+
$this->assertSame('foo', url('foo'));
179+
}
180+
181+
/** @covers ::url */
182+
public function testUrlFunctionWithoutBaseUrlOrPath()
183+
{
184+
$this->app['config']->set(['hyde.url' => null]);
185+
$this->expectException(\Hyde\Framework\Exceptions\BaseUrlNotSetException::class);
186+
$this->assertNull(url());
187+
}
188+
189+
/** @covers ::url */
190+
public function testUrlFunctionWithLocalhostBaseUrlButNoPath()
191+
{
192+
$this->app['config']->set(['hyde.url' => 'http://localhost']);
157193
$this->expectException(\Hyde\Framework\Exceptions\BaseUrlNotSetException::class);
158-
$this->assertNull(url('foo'));
194+
$this->assertNull(url());
159195
}
160196

161197
/** @covers ::url */

‎packages/framework/tests/Feature/SitesWithoutBaseUrlAreHandledGracefullyTest.php‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Hyde\Framework\Testing\Feature;
6+
7+
use Hyde\Hyde;
8+
use Hyde\Testing\TestCase;
9+
use Hyde\Pages\MarkdownPage;
10+
use Hyde\Pages\MarkdownPost;
11+
use Hyde\Pages\DocumentationPage;
12+
13+
/**
14+
* High level test to ensure that sites without a base URL are handled gracefully.
15+
*
16+
* For example: In case a user forgot to set a base URL, we don't want their site
17+
* to have localhost links in the compiled HTML output since that would break
18+
* things when deployed to production. So we fall back to relative links.
19+
*
20+
* Some things like sitemaps and RSS feeds cannot be generated without a base URL,
21+
* as their schemas generally do not allow relative URLs. In those cases, we
22+
* don't generate files at all, and we don't add any links to them either.
23+
*
24+
* @coversNothing This test is not testing a specific class, but a general feature of the framework.
25+
*/
26+
class SitesWithoutBaseUrlAreHandledGracefullyTest extends TestCase
27+
{
28+
public static function pageClassProvider(): array
29+
{
30+
return [
31+
[MarkdownPage::class],
32+
[MarkdownPost::class],
33+
[DocumentationPage::class],
34+
];
35+
}
36+
37+
/** @dataProvider pageClassProvider */
38+
public function testLocalhostLinksAreNotAddedToCompiledHtmlWhenBaseUrlIsNull(string $class)
39+
{
40+
config(['hyde.url' => null]);
41+
42+
$this->assertStringNotContainsString('http://localhost', $this->getHtml($class));
43+
}
44+
45+
/** @dataProvider pageClassProvider */
46+
public function testLocalhostLinksAreNotAddedToCompiledHtmlWhenBaseUrlIsNotSet(string $class)
47+
{
48+
config(['hyde.url' => '']);
49+
50+
$this->assertStringNotContainsString('http://localhost', $this->getHtml($class));
51+
}
52+
53+
/** @dataProvider pageClassProvider */
54+
public function testLocalhostLinksAreNotAddedToCompiledHtmlWhenBaseUrlIsSetToLocalhost(string $class)
55+
{
56+
config(['hyde.url' => 'http://localhost']);
57+
58+
$this->assertStringNotContainsString('http://localhost', $this->getHtml($class));
59+
}
60+
61+
/** @dataProvider pageClassProvider */
62+
public function testSiteUrlLinksAreAddedToCompiledHtmlWhenBaseUrlIsSetToValidUrl(string $class)
63+
{
64+
config(['hyde.url' => 'https://example.com']);
65+
66+
$this->assertStringNotContainsString('http://localhost', $this->getHtml($class));
67+
}
68+
69+
protected function getHtml(string $class): string
70+
{
71+
$page = new $class('foo');
72+
73+
Hyde::shareViewData($page);
74+
75+
return $page->compile();
76+
}
77+
}

0 commit comments

Comments
 (0)