Skip to content

Commit

Permalink
Fix IDN domain name not being allowed
Browse files Browse the repository at this point in the history
The filter_var function is unfortunately not perfect and doesn't support
domain with unicode as well as url with underscores. Replace usage with
a regex.

See https://bugs.php.net/search.php?cmd=display&search_for=FILTER_VALIDATE_URL

Closes #27906

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
  • Loading branch information
CarlSchwan committed Jul 14, 2021
1 parent 2a0bd66 commit 24db036
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 14 deletions.
2 changes: 1 addition & 1 deletion apps/oauth2/lib/Controller/SettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public function __construct(string $appName,

public function addClient(string $name,
string $redirectUri): JSONResponse {
if (filter_var($redirectUri, FILTER_VALIDATE_URL) === false) {
if (!\OCP\Util::isValidUrl($redirectUri)) {
return new JSONResponse(['message' => $this->l->t('Your redirect URL needs to be a full URL for example: https://yourdomain.com/path')], Http::STATUS_BAD_REQUEST);
}

Expand Down
15 changes: 4 additions & 11 deletions apps/theming/lib/Controller/ThemingController.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
use OCP\IRequest;
use OCP\ITempManager;
use OCP\IURLGenerator;
use OCP\Util;

/**
* Class ThemingController
Expand Down Expand Up @@ -142,23 +143,23 @@ public function updateStylesheet($setting, $value) {
if (strlen($value) > 500) {
$error = $this->l10n->t('The given web address is too long');
}
if (!$this->isValidUrl($value)) {
if (!Util::isValidUrl($value)) {
$error = $this->l10n->t('The given web address is not a valid URL');
}
break;
case 'imprintUrl':
if (strlen($value) > 500) {
$error = $this->l10n->t('The given legal notice address is too long');
}
if (!$this->isValidUrl($value)) {
if (!Util::isValidUrl($value)) {
$error = $this->l10n->t('The given legal notice address is not a valid URL');
}
break;
case 'privacyUrl':
if (strlen($value) > 500) {
$error = $this->l10n->t('The given privacy policy address is too long');
}
if (!$this->isValidUrl($value)) {
if (!Util::isValidUrl($value)) {
$error = $this->l10n->t('The given privacy policy address is not a valid URL');
}
break;
Expand Down Expand Up @@ -199,14 +200,6 @@ public function updateStylesheet($setting, $value) {
);
}

/**
* Check that a string is a valid http/https url
*/
private function isValidUrl(string $url): bool {
return ((strpos($url, 'http://') === 0 || strpos($url, 'https://') === 0) &&
filter_var($url, FILTER_VALIDATE_URL) !== false);
}

/**
* @return DataResponse
* @throws NotPermittedException
Expand Down
2 changes: 1 addition & 1 deletion apps/theming/lib/ThemingDefaults.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public function getShortFooter() {
$divider = '';
foreach ($links as $link) {
if ($link['url'] !== ''
&& filter_var($link['url'], FILTER_VALIDATE_URL)
&& \OCP\Util::isValidUrl($link['url'])
) {
$legalLinks .= $divider . '<a href="' . $link['url'] . '" class="legal" target="_blank"' .
' rel="noreferrer noopener">' . $link['text'] . '</a>';
Expand Down
2 changes: 1 addition & 1 deletion lib/private/Setup.php
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ private static function findWebRoot(SystemConfig $config): string {
if ($webRoot === '') {
throw new InvalidArgumentException('overwrite.cli.url is empty');
}
if (!filter_var($webRoot, FILTER_VALIDATE_URL)) {
if (!\OCP\Util::isValidUrl($webRoot)) {
throw new InvalidArgumentException('invalid value for overwrite.cli.url');
}
$webRoot = rtrim(parse_url($webRoot, PHP_URL_PATH), '/');
Expand Down
12 changes: 12 additions & 0 deletions lib/public/Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -513,4 +513,16 @@ public static function needUpgrade() {
}
return self::$needUpgradeCache;
}

/**
* Checks whether the given URL is valid. This function should be
* used instead of filter_var($url, FILTER_VALIDATE_URL) since it
* handles idn urls too.
*
* @return bool true if the url is valid, false otherwise.
* @since 23.0.0
*/
public static function isValidUrl(string $url): bool {
return filter_var(idn_to_ascii($url), FILTER_VALIDATE_URL) !== false;
}
}

0 comments on commit 24db036

Please sign in to comment.