Skip to content

fix(ReferenceProvider): tolerate http/https scheme missmatch #2558

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.

Sign up for GitHub

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

Open
mejo- wants to merge 1 commit into
base: main
Choose a base branch
from
+58 −8
Open

fix(ReferenceProvider): tolerate http/https scheme missmatch #2558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions lib/Reference/SearchablePageReferenceProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

use DateTime;
use Exception;
use OC\Collaboration\Reference\LinkReferenceProvider;
use OC\Collaboration\Reference\ReferenceManager;
use OCA\Collectives\AppInfo\Application;
use OCA\Collectives\Db\Collective;
Expand All @@ -26,6 +25,7 @@
use OCP\Collaboration\Reference\IPublicReferenceProvider;
use OCP\Collaboration\Reference\IReference;
use OCP\Collaboration\Reference\ISearchableReferenceProvider;
use OCP\Collaboration\Reference\LinkReferenceProvider;
use OCP\Collaboration\Reference\Reference;
use OCP\IDateTimeFormatter;
use OCP\IL10N;
Expand Down Expand Up @@ -106,6 +106,13 @@ private static function pagePathFromMatches(array $urlParts, string $collectiveP
return $pagePath;
}

private function quoteBaseUrl(string $path): string {
$url = $this->urlGenerator->getAbsoluteURL($path);
// Drop http/https scheme so we can quote the rest and tolerate http/https mismatches (e.g. reverse proxies without `overwriteprotocol`)
$afterScheme = preg_replace('#^https?://#', '', $url);
return 'https?:\/\/' . preg_quote($afterScheme, '/');
}

public function matchUrl(string $url): ?array {
$urlParts = parse_url($url);
if (!$urlParts || !array_key_exists('path', $urlParts) || empty($urlParts['path'])) {
Expand All @@ -119,13 +126,13 @@ public function matchUrl(string $url): ?array {
// https://nextcloud.local/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective/Tutos/Hacking/Spectre
// https://nextcloud.local/index.php/apps/collectives/p/supacollective/MsdwSCmP9F6jcQX/Tutos/Hacking/Spectre?fileId=14457
$startPublicRegexes = [
$this->urlGenerator->getAbsoluteURL('/apps/' . Application::APP_NAME . '/p'),
$this->urlGenerator->getAbsoluteURL('/index.php/apps/' . Application::APP_NAME . '/p'),
$this->quoteBaseUrl('/apps/' . Application::APP_NAME . '/p'),
$this->quoteBaseUrl('/index.php/apps/' . Application::APP_NAME . '/p'),
];

$matches = false;
foreach ($startPublicRegexes as $regex) {
preg_match('/^' . preg_quote($regex, '/') . '\/\w+' . '\/([^\/]+)(\/[^?#]+)?/i', $url, $matches);
preg_match('/^' . $regex . '\/\w+' . '\/([^\/]+)(\/[^?#]+)?/i', $url, $matches);
if ($matches && count($matches) > 1) {
return self::pagePathFromMatches($urlParts, $matches[1], $matches[2] ?? '');
}
Expand All @@ -137,12 +144,12 @@ public function matchUrl(string $url): ?array {
// https://nextcloud.local/apps/collectives/supacollective/Tutos/Hacking/Spectre?fileId=14457
// https://nextcloud.local/apps/collectives/supacollective/Tutos/Hacking/Spectre
$startRegexes = [
$this->urlGenerator->getAbsoluteURL('/apps/' . Application::APP_NAME),
$this->urlGenerator->getAbsoluteURL('/index.php/apps/' . Application::APP_NAME),
$this->quoteBaseUrl('/apps/' . Application::APP_NAME),
$this->quoteBaseUrl('/index.php/apps/' . Application::APP_NAME),
];

foreach ($startRegexes as $regex) {
preg_match('/^' . preg_quote($regex, '/') . '\/([^\/]+)(\/[^?#]+)?/i', $url, $matches);
preg_match('/^' . $regex . '\/([^\/]+)(\/[^?#]+)?/i', $url, $matches);
if ($matches && count($matches) > 1) {
return self::pagePathFromMatches($urlParts, $matches[1], $matches[2] ?? '');
}
Expand Down
45 changes: 44 additions & 1 deletion tests/Unit/Search/SearchablePageReferenceProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@

namespace Unit\Search;

use OC\Collaboration\Reference\LinkReferenceProvider;
use OC\Collaboration\Reference\ReferenceManager;
use OCA\Collectives\Reference\SearchablePageReferenceProvider;
use OCA\Collectives\Service\CollectiveService;
use OCA\Collectives\Service\CollectiveShareService;
use OCA\Collectives\Service\PageService;
use OCA\Collectives\Service\SharePageService;
use OCP\Collaboration\Reference\IPublicReferenceProvider;
use OCP\Collaboration\Reference\LinkReferenceProvider;
use OCP\IDateTimeFormatter;
use OCP\IL10N;
use OCP\IURLGenerator;
Expand Down Expand Up @@ -70,10 +70,14 @@ private function slugUrlProvider(): array {
// internal
['https://nextcloud.local/apps/collectives/supacollective-123/spectre-slug-14457', 'spectre-slug-14457', null],
['https://nextcloud.local/apps/collectives/supacollective-123/spectre-slug-14457#h-heading1', 'spectre-slug-14457', 'h-heading1'],
// scheme mismatch: server reports https, link is http
['http://nextcloud.local/apps/collectives/supacollective-123/spectre-slug-14457', 'spectre-slug-14457', null],

// public
['https://nextcloud.local/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective-123/spectre-slug-14457', 'spectre-slug-14457', null],
['https://nextcloud.local/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective-123/spectre-slug-14457#h-heading1', 'spectre-slug-14457', 'h-heading1'],
// scheme mismatch: server reports https, link is http
['http://nextcloud.local/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective-123/spectre-slug-14457', 'spectre-slug-14457', null],
];
}

Expand All @@ -84,13 +88,17 @@ private function urlProvider(): array {
['https://nextcloud.local/apps/collectives/supacollective/Tutos/Hacking/Spectre', 'Tutos/Hacking/Spectre', null],
['https://nextcloud.local/apps/collectives/supacollective/Tutos/Hacking/Spectre#h-heading1', 'Tutos/Hacking/Spectre', 'h-heading1'],
['https://nextcloud.local/index.php/apps/collectives/supacollective/Tutos/Hacking/Spectre', 'Tutos/Hacking/Spectre', null],
// scheme mismatch: server reports https, link is http
['http://nextcloud.local/apps/collectives/supacollective/Tutos/Hacking/Spectre', 'Tutos/Hacking/Spectre', null],

// public
['https://nextcloud.local/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective', '', null],
['https://nextcloud.local/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective/abc', 'abc', null],
['https://nextcloud.local/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective/Tutos/Hacking/Spectre', 'Tutos/Hacking/Spectre', null],
['https://nextcloud.local/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective/Tutos/Hacking/Spectre#h-heading1', 'Tutos/Hacking/Spectre', 'h-heading1'],
['https://nextcloud.local/index.php/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective/Tutos/Hacking/Spectre', 'Tutos/Hacking/Spectre', null],
// scheme mismatch: server reports https, link is http
['http://nextcloud.local/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective/Tutos/Hacking/Spectre', 'Tutos/Hacking/Spectre', null],
];
}

Expand Down Expand Up @@ -165,6 +173,41 @@ public function testMatchUrlNonMatching(string $url): void {
self::assertEquals(null, $this->provider->matchUrl($url));
}

public function testMatchUrlServerHttpUserHttps(): void {
$urlGenerator = $this->createMock(IURLGenerator::class);
$urlGenerator->method('getAbsoluteURL')->willReturnMap([
['/apps/collectives/p', 'http://nextcloud.local/apps/collectives/p'],
['/index.php/apps/collectives/p', 'http://nextcloud.local/index.php/apps/collectives/p'],
['/apps/collectives', 'http://nextcloud.local/apps/collectives'],
['/index.php/apps/collectives', 'http://nextcloud.local/index.php/apps/collectives'],
]);
$provider = new SearchablePageReferenceProvider(
$this->collectiveService,
$this->createMock(PageService::class),
$this->createMock(SharePageService::class),
$this->createMock(IL10N::class),
$urlGenerator,
$this->createMock(IDateTimeFormatter::class),
$this->createMock(ReferenceManager::class),
$this->createMock(LinkReferenceProvider::class),
$this->collectiveShareService,
'jane',
);

$expected = [
'collectiveName' => 'supacollective',
'pagePath' => 'Tutos/Hacking/Spectre',
'fragment' => null,
];

// server reports http://, link is https://
self::assertEquals($expected, $provider->matchUrl('https://nextcloud.local/apps/collectives/supacollective/Tutos/Hacking/Spectre'));
// server reports http://, link is http://
self::assertEquals($expected, $provider->matchUrl('http://nextcloud.local/apps/collectives/supacollective/Tutos/Hacking/Spectre'));
// public share: server reports http://, link is https://
self::assertEquals($expected, $provider->matchUrl('https://nextcloud.local/apps/collectives/p/MsdwSCmP9F6jcQX/supacollective/Tutos/Hacking/Spectre'));
}

public function testResolveReferencePublicNotAuthenticated(): void {
$this->collectiveShareService
->method('isShareAuthenticated')
Expand Down
Loading