Skip to content

Commit 2d56f53

Browse files
committed
fix: use an indepentant URL for checking internet connectivtiy
1 parent a9d3918 commit 2d56f53

4 files changed

Lines changed: 53 additions & 119 deletions

File tree

changelog/unreleased/41506

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
Change: Use configurable URL for internet connectivity check
2+
3+
Default URL is now configurable and the default is set to an independent
4+
resource: https://detectportal.firefox.com/success.txt
5+
This also provides an IPv6 compatible URL.
6+
7+
https://github.com/owncloud/core/pull/41506
8+
https://github.com/owncloud/core/issues/41465

config/config.sample.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,10 @@
758758
*/
759759
'has_internet_connection' => true,
760760

761+
/**
762+
* The following URL is used to detect internet connectivity.
763+
*/
764+
'internet_connectivity_detect_url' => 'https://detectportal.firefox.com/success.txt',
761765
/**
762766
* Check for a `.well-known` setup
763767
* Allows ownCloud to verify a working .well-known URL redirect.

settings/Controller/CheckSetupController.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ private function isInternetConnectionWorking(): bool {
8383
}
8484

8585
try {
86+
$detectUrl = $this->config->getSystemValue('internet_connectivity_detect_url', 'https://detectportal.firefox.com/success.txt');
8687
$client = $this->clientService->newClient();
87-
$client->get('https://www.owncloud.com/');
88-
$client->get('http://www.owncloud.com/');
88+
$client->get($detectUrl);
8989
return true;
9090
} catch (\Exception $e) {
9191
return false;

tests/Settings/Controller/CheckSetupControllerTest.php

Lines changed: 39 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
use OCP\IRequest;
3535
use OCP\IURLGenerator;
3636
use Test\TestCase;
37+
use OCP\Http\Client\IClient;
3738

3839
/**
3940
* Class CheckSetupControllerTest
@@ -59,24 +60,24 @@ class CheckSetupControllerTest extends TestCase {
5960
public function setUp(): void {
6061
parent::setUp();
6162

62-
$this->request = $this->getMockBuilder('\OCP\IRequest')
63+
$this->request = $this->getMockBuilder(IRequest::class)
6364
->disableOriginalConstructor()->getMock();
64-
$this->config = $this->getMockBuilder('\OCP\IConfig')
65+
$this->config = $this->getMockBuilder(IConfig::class)
6566
->disableOriginalConstructor()->getMock();
66-
$this->clientService = $this->getMockBuilder('\OCP\Http\Client\IClientService')
67+
$this->clientService = $this->getMockBuilder(IClientService::class)
6768
->disableOriginalConstructor()->getMock();
68-
$this->urlGenerator = $this->getMockBuilder('\OCP\IURLGenerator')
69+
$this->urlGenerator = $this->getMockBuilder(IURLGenerator::class)
6970
->disableOriginalConstructor()->getMock();
70-
$this->l10n = $this->getMockBuilder('\OCP\IL10N')
71+
$this->l10n = $this->getMockBuilder(IL10N::class)
7172
->disableOriginalConstructor()->getMock();
7273
$this->l10n
7374
->method('t')
7475
->willReturnCallback(function ($message, array $replace) {
7576
return \vsprintf($message, $replace);
7677
});
77-
$this->checker = $this->getMockBuilder('\OC\IntegrityCheck\Checker')
78+
$this->checker = $this->getMockBuilder(Checker::class)
7879
->disableOriginalConstructor()->getMock();
79-
$this->checkSetupController = $this->getMockBuilder('\OC\Settings\Controller\CheckSetupController')
80+
$this->checkSetupController = $this->getMockBuilder(CheckSetupController::class)
8081
->setConstructorArgs([
8182
'settings',
8283
$this->request,
@@ -103,92 +104,6 @@ public function testIsInternetConnectionWorkingDisabledViaConfig() {
103104
);
104105
}
105106

106-
public function testIsInternetConnectionWorkingCorrectly() {
107-
$this->config->expects($this->once())
108-
->method('getSystemValue')
109-
->with('has_internet_connection', true)
110-
->willReturn(true);
111-
112-
$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
113-
->disableOriginalConstructor()->getMock();
114-
$client
115-
->expects($this->exactly(2))
116-
->method('get')
117-
->withConsecutive(
118-
['https://www.owncloud.com/', []],
119-
['http://www.owncloud.com/', []],
120-
);
121-
122-
$this->clientService->expects($this->once())
123-
->method('newClient')
124-
->willReturn($client);
125-
126-
$this->assertTrue(
127-
self::invokePrivate(
128-
$this->checkSetupController,
129-
'isInternetConnectionWorking'
130-
)
131-
);
132-
}
133-
134-
public function testIsInternetConnectionHttpsFail() {
135-
$this->config->expects($this->once())
136-
->method('getSystemValue')
137-
->with('has_internet_connection', true)
138-
->willReturn(true);
139-
140-
$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
141-
->disableOriginalConstructor()->getMock();
142-
$client
143-
->expects($this->once())
144-
->method('get')
145-
->with('https://www.owncloud.com/', [])
146-
->will($this->throwException(new \Exception()));
147-
148-
$this->clientService->expects($this->once())
149-
->method('newClient')
150-
->willReturn($client);
151-
152-
$this->assertFalse(
153-
self::invokePrivate(
154-
$this->checkSetupController,
155-
'isInternetConnectionWorking'
156-
)
157-
);
158-
}
159-
160-
public function testIsInternetConnectionHttpFail() {
161-
$this->config->expects($this->once())
162-
->method('getSystemValue')
163-
->with('has_internet_connection', true)
164-
->willReturn(true);
165-
166-
$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
167-
->disableOriginalConstructor()->getMock();
168-
$client
169-
->expects($this->exactly(2))
170-
->method('get')
171-
->withConsecutive(
172-
['https://www.owncloud.com/', []],
173-
['http://www.owncloud.com/', []],
174-
)
175-
->willReturnOnConsecutiveCalls(
176-
[],
177-
$this->throwException(new \Exception()),
178-
);
179-
180-
$this->clientService->expects($this->once())
181-
->method('newClient')
182-
->willReturn($client);
183-
184-
$this->assertFalse(
185-
self::invokePrivate(
186-
$this->checkSetupController,
187-
'isInternetConnectionWorking'
188-
)
189-
);
190-
}
191-
192107
public function testIsMemcacheConfiguredFalse() {
193108
$this->config->expects($this->once())
194109
->method('getSystemValue')
@@ -280,18 +195,22 @@ public function testForwardedForHeadersWorkingTrue() {
280195
);
281196
}
282197

283-
public function testCheck() {
198+
/**
199+
* @dataProvider providesCheckData
200+
*/
201+
public function testCheck(bool $internet_available) {
284202
$this->config
285-
->expects($this->any())
286203
->method('getSystemValue')
287204
->withConsecutive(
288205
['has_internet_connection', true],
206+
['internet_connectivity_detect_url', 'https://detectportal.firefox.com/success.txt'],
289207
['memcache.local', null],
290208
['has_internet_connection', true],
291209
['trusted_proxies', []],
292210
)
293211
->willReturnOnConsecutiveCalls(
294212
true,
213+
'https://detectportal.firefox.com/success.txt',
295214
'SomeProvider',
296215
false,
297216
['1.2.3.4'],
@@ -301,19 +220,17 @@ public function testCheck() {
301220
->method('getRemoteAddress')
302221
->willReturn('4.3.2.1');
303222

304-
$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
223+
$client = $this->getMockBuilder(IClient::class)
305224
->disableOriginalConstructor()->getMock();
306225
$client
307-
->expects($this->exactly(2))
226+
->expects($this->once())
308227
->method('get')
309-
->withConsecutive(
310-
['https://www.owncloud.com/', []],
311-
['http://www.owncloud.com/', []],
312-
)
313-
->willReturnOnConsecutiveCalls(
314-
[],
315-
$this->throwException(new \Exception())
316-
);
228+
->willReturnCallback(function () use ($internet_available) {
229+
if ($internet_available) {
230+
return;
231+
}
232+
throw new \Exception();
233+
});
317234

318235
$this->clientService->expects($this->once())
319236
->method('newClient')
@@ -339,7 +256,7 @@ public function testCheck() {
339256

340257
$expected = new DataResponse(
341258
[
342-
'serverHasInternetConnection' => false,
259+
'serverHasInternetConnection' => $internet_available,
343260
'isMemcacheConfigured' => true,
344261
'memcacheDocs' => 'http://doc.owncloud.com/server/go.php?to=admin-performance',
345262
'isUrandomAvailable' => self::invokePrivate($this->checkSetupController, 'isUrandomAvailable'),
@@ -361,7 +278,7 @@ public function testCheck() {
361278
}
362279

363280
public function testGetCurlVersion() {
364-
$checkSetupController = $this->getMockBuilder('\OC\Settings\Controller\CheckSetupController')
281+
$checkSetupController = $this->getMockBuilder(CheckSetupController::class)
365282
->setConstructorArgs([
366283
'settings',
367284
$this->request,
@@ -377,7 +294,7 @@ public function testGetCurlVersion() {
377294
}
378295

379296
public function testIsUsedTlsLibOutdatedWithAnotherLibrary() {
380-
$this->config->expects($this->any())
297+
$this->config
381298
->method('getSystemValue')
382299
->willReturn(true);
383300
$this->checkSetupController
@@ -388,7 +305,7 @@ public function testIsUsedTlsLibOutdatedWithAnotherLibrary() {
388305
}
389306

390307
public function testIsUsedTlsLibOutdatedWithMisbehavingCurl() {
391-
$this->config->expects($this->any())
308+
$this->config
392309
->method('getSystemValue')
393310
->willReturn(true);
394311
$this->checkSetupController
@@ -399,7 +316,7 @@ public function testIsUsedTlsLibOutdatedWithMisbehavingCurl() {
399316
}
400317

401318
public function testIsUsedTlsLibOutdatedWithOlderOpenSsl() {
402-
$this->config->expects($this->any())
319+
$this->config
403320
->method('getSystemValue')
404321
->willReturn(true);
405322
$this->checkSetupController
@@ -410,7 +327,7 @@ public function testIsUsedTlsLibOutdatedWithOlderOpenSsl() {
410327
}
411328

412329
public function testIsUsedTlsLibOutdatedWithOlderOpenSsl1() {
413-
$this->config->expects($this->any())
330+
$this->config
414331
->method('getSystemValue')
415332
->willReturn(true);
416333
$this->checkSetupController
@@ -421,7 +338,7 @@ public function testIsUsedTlsLibOutdatedWithOlderOpenSsl1() {
421338
}
422339

423340
public function testIsUsedTlsLibOutdatedWithMatchingOpenSslVersion() {
424-
$this->config->expects($this->any())
341+
$this->config
425342
->method('getSystemValue')
426343
->willReturn(true);
427344
$this->checkSetupController
@@ -432,7 +349,7 @@ public function testIsUsedTlsLibOutdatedWithMatchingOpenSslVersion() {
432349
}
433350

434351
public function testIsUsedTlsLibOutdatedWithMatchingOpenSslVersion1() {
435-
$this->config->expects($this->any())
352+
$this->config
436353
->method('getSystemValue')
437354
->willReturn(true);
438355
$this->checkSetupController
@@ -443,14 +360,14 @@ public function testIsUsedTlsLibOutdatedWithMatchingOpenSslVersion1() {
443360
}
444361

445362
public function testIsBuggyNss400() {
446-
$this->config->expects($this->any())
363+
$this->config
447364
->method('getSystemValue')
448365
->willReturn(true);
449366
$this->checkSetupController
450367
->expects($this->once())
451368
->method('getCurlVersion')
452369
->willReturn(['ssl_version' => 'NSS/1.0.2b']);
453-
$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
370+
$client = $this->getMockBuilder(IClient::class)
454371
->disableOriginalConstructor()->getMock();
455372
/** @var ClientException | \PHPUnit\Framework\MockObject\MockObject $exception */
456373
$exception = $this->getMockBuilder('\GuzzleHttp\Exception\ClientException')
@@ -478,14 +395,14 @@ public function testIsBuggyNss400() {
478395
}
479396

480397
public function testIsBuggyNss200() {
481-
$this->config->expects($this->any())
398+
$this->config
482399
->method('getSystemValue')
483400
->willReturn(true);
484401
$this->checkSetupController
485402
->expects($this->once())
486403
->method('getCurlVersion')
487404
->willReturn(['ssl_version' => 'NSS/1.0.2b']);
488-
$client = $this->getMockBuilder('\OCP\Http\Client\IClient')
405+
$client = $this->getMockBuilder(IClient::class)
489406
->disableOriginalConstructor()->getMock();
490407
/** @var ClientException | \PHPUnit\Framework\MockObject\MockObject $exception */
491408
$exception = $this->getMockBuilder('\GuzzleHttp\Exception\ClientException')
@@ -1018,4 +935,9 @@ public function testGetFailedIntegrityCheckFilesWithSomeErrorsFound() {
1018935
);
1019936
$this->assertEquals($expected, $this->checkSetupController->getFailedIntegrityCheckFiles());
1020937
}
938+
939+
public function providesCheckData() {
940+
yield [true];
941+
yield [false];
942+
}
1021943
}

0 commit comments

Comments
 (0)