diff --git a/src/yunohost/domain.py b/src/yunohost/domain.py
index 3848213d2..8ef7bb390 100644
--- a/src/yunohost/domain.py
+++ b/src/yunohost/domain.py
@@ -395,7 +395,7 @@ def _normalize_domain_path(domain, path):
return domain, path
-def _check_and_normalize_permission_path(url):
+def _check_and_sanitize_permission_path(url, app_main_path, permission):
"""
Check and normalize the urls passed for all permissions
Also check that the Regex is valid
@@ -416,48 +416,77 @@ def _check_and_normalize_permission_path(url):
"""
import re, sre_constants
- # Uri without domain
- if url.startswith('re:/'):
- regex = url[4:]
- # check regex
- try:
- re.compile(regex)
- except sre_constants.error:
- raise YunohostError('invalid_regex', regex=regex)
- return url
-
- if url.startswith('/'):
- return '/' + url.strip("/")
-
# Uri with domain
domains = domain_list()['domains']
+ # regex without domain
+ if url.startswith('re:/'):
+ regex = url[4:]
+ # check regex if it's a PCRE regex, if it's a lua regex just print a warning
+ # I don't how how to validate a regex
+ if '%' in regex:
+ logger.warning("/!\\ Packagers! You are probably using a lua regex. You should use a PCRE regex instead.")
+ else:
+ try:
+ re.compile(regex)
+ except sre_constants.error:
+ raise YunohostError('invalid_regex', regex=regex)
+ return url
+
+ # regex with domain
if url.startswith('re:'):
if '/' not in url:
raise YunohostError('regex_with_only_domain')
domain = url[3:].split('/')[0]
path = '/' + url[3:].split('/', 1)[1]
- if domain not in domains:
+ if domain.replace('%', '').replace('\\', '') not in domains:
raise YunohostError('domain_named_unknown', domain=domain)
- try:
- re.compile(path)
- except sre_constants.error:
- raise YunohostError('invalid_regex', regex=path)
+ if '%' in path:
+ logger.warning("/!\\ Packagers! You are probably using a lua regex. You should use a PCRE regex instead.")
+ else:
+ try:
+ re.compile(path)
+ except sre_constants.error:
+ raise YunohostError('invalid_regex', regex=path)
return 're:' + domain + path
+ # uris without domain
+ if url.startswith('/'):
+ sanitized_url = '/' + url.strip("/")
+ domain = app_main_path.split('/')[0]
+ path = ('/' + app_main_path.split('/')[1]) if '/' in app_main_path else '/'
+
+ # uris with domain
else:
domain = url.split('/')[0]
if domain not in domains:
raise YunohostError('domain_named_unknown', domain=domain)
if '/' in url:
- path = url.split('/', 1)[1].rstrip('/')
- return domain + '/' + path
+ path = '/' + url.split('/', 1)[1].rstrip('/')
+ sanitized_url = domain + path
else:
- return domain
+ sanitized_url = domain
+ path = '/'
+
+ conflicts = _get_conflicting_apps(domain, path, ignore_app=permission.split('.')[0])
+
+ if conflicts:
+ apps = []
+ for path, app_id, app_label in conflicts:
+ apps.append(" * {domain:s}{path:s} → {app_label:s} ({app_id:s})".format(
+ domain=domain,
+ path=path,
+ app_id=app_id,
+ app_label=app_label,
+ ))
+
+ raise YunohostError('app_location_unavailable', apps="\n".join(apps))
+
+ return sanitized_url
def _build_dns_conf(domain, ttl=3600):
diff --git a/src/yunohost/permission.py b/src/yunohost/permission.py
index 387b04858..e3d912047 100644
--- a/src/yunohost/permission.py
+++ b/src/yunohost/permission.py
@@ -364,7 +364,7 @@ def permission_url(operation_logger, permission,
"""
from yunohost.app import app_setting
from yunohost.utils.ldap import _get_ldap_interface
- from yunohost.domain import _check_and_normalize_permission_path, _get_conflicting_apps
+ from yunohost.domain import _check_and_sanitize_permission_path, _get_conflicting_apps
ldap = _get_ldap_interface()
# By default, manipulate main permission
@@ -389,26 +389,12 @@ def permission_url(operation_logger, permission,
if url is None:
url = existing_permission["url"]
else:
- url = _check_and_normalize_permission_path(url)
- domain, path = _get_full_url(url, app_main_path).split('/', 1)
- domain = domain[3:] if domain.startswith("re:") else domain
- conflicts = _get_conflicting_apps(domain, path, ignore_app=permission.split('.')[0])
+ url = _check_and_sanitize_permission_path(url, app_main_path, permission)
+
if url.startswith('re:') and existing_permission['show_tile']:
logger.warning(m18n.n('regex_incompatible_with_tile', regex=url, permission=permission))
show_tile = False
- if conflicts:
- apps = []
- for path, app_id, app_label in conflicts:
- apps.append(" * {domain:s}{path:s} → {app_label:s} ({app_id:s})".format(
- domain=domain,
- path=path,
- app_id=app_id,
- app_label=app_label,
- ))
-
- raise YunohostError('app_location_unavailable', apps="\n".join(apps))
-
current_additional_urls = existing_permission["additional_urls"]
new_additional_urls = copy.copy(current_additional_urls)
@@ -417,22 +403,7 @@ def permission_url(operation_logger, permission,
if ur in current_additional_urls:
logger.warning(m18n.n('additional_urls_already_added', permission=permission, url=ur))
else:
- ur = _check_and_normalize_permission_path(ur)
- domain, path = _get_full_url(ur, app_main_path).split('/', 1)
- domain = domain[3:] if domain.startswith("re:") else domain
- conflicts = _get_conflicting_apps(domain, path, ignore_app=permission.split('.')[0])
-
- if conflicts:
- apps = []
- for path, app_id, app_label in conflicts:
- apps.append(" * {domain:s}{path:s} → {app_label:s} ({app_id:s})".format(
- domain=domain,
- path=path,
- app_id=app_id,
- app_label=app_label,
- ))
-
- raise YunohostError('app_location_unavailable', apps="\n".join(apps))
+ ur = _check_and_sanitize_permission_path(ur, app_main_path, permission)
new_additional_urls += [ur]
if remove_url:
@@ -657,6 +628,6 @@ def _get_full_url(url, app_main_path):
if url.startswith('/'):
return app_main_path + url.rstrip("/")
if url.startswith('re:/'):
- return 're:' + app_main_path + url[3:]
+ return 're:' + app_main_path.replace('.', '\\.') + url[3:]
else:
return url
diff --git a/src/yunohost/tests/test_appurl.py b/src/yunohost/tests/test_appurl.py
index 3b7af4d92..e7df12a12 100644
--- a/src/yunohost/tests/test_appurl.py
+++ b/src/yunohost/tests/test_appurl.py
@@ -2,7 +2,7 @@ import pytest
from yunohost.utils.error import YunohostError
from yunohost.app import app_install, app_remove
-from yunohost.domain import _get_maindomain, domain_url_available, _normalize_domain_path, _check_and_normalize_permission_path
+from yunohost.domain import _get_maindomain, domain_url_available, _normalize_domain_path, _check_and_sanitize_permission_path
# Get main domain
maindomain = _get_maindomain()
@@ -63,40 +63,50 @@ def test_registerurl_baddomain():
def test_normalize_permission_path():
# Relative path
- assert _check_and_normalize_permission_path("/wiki/") == "/wiki"
- assert _check_and_normalize_permission_path("/") == "/"
- assert _check_and_normalize_permission_path("//salut/") == "/salut"
+ assert _check_and_sanitize_permission_path("/wiki/", maindomain + '/path', 'test_permission.main') == "/wiki"
+ assert _check_and_sanitize_permission_path("/", maindomain + '/path', 'test_permission.main') == "/"
+ assert _check_and_sanitize_permission_path("//salut/", maindomain + '/path', 'test_permission.main') == "/salut"
# Full path
- assert _check_and_normalize_permission_path(maindomain + "/hey/") == maindomain + "/hey"
- assert _check_and_normalize_permission_path(maindomain + "//") == maindomain + "/"
- assert _check_and_normalize_permission_path(maindomain + "/") == maindomain + "/"
+ assert _check_and_sanitize_permission_path(maindomain + "/hey/", maindomain + '/path', 'test_permission.main') == maindomain + "/hey"
+ assert _check_and_sanitize_permission_path(maindomain + "//", maindomain + '/path', 'test_permission.main') == maindomain + "/"
+ assert _check_and_sanitize_permission_path(maindomain + "/", maindomain + '/path', 'test_permission.main') == maindomain + "/"
# Relative Regex
- assert _check_and_normalize_permission_path("re:/yolo.*/") == "re:/yolo.*/"
- assert _check_and_normalize_permission_path("re:/y.*o(o+)[a-z]*/bo\1y") == "re:/y.*o(o+)[a-z]*/bo\1y"
+ assert _check_and_sanitize_permission_path("re:/yolo.*/", maindomain + '/path', 'test_permission.main') == "re:/yolo.*/"
+ assert _check_and_sanitize_permission_path("re:/y.*o(o+)[a-z]*/bo\1y", maindomain + '/path', 'test_permission.main') == "re:/y.*o(o+)[a-z]*/bo\1y"
# Full Regex
- assert _check_and_normalize_permission_path("re:" + maindomain + "/yolo.*/") == "re:" + maindomain + "/yolo.*/"
- assert _check_and_normalize_permission_path("re:" + maindomain + "/y.*o(o+)[a-z]*/bo\1y") == "re:" + maindomain + "/y.*o(o+)[a-z]*/bo\1y"
+ assert _check_and_sanitize_permission_path("re:" + maindomain + "/yolo.*/", maindomain + '/path', 'test_permission.main') == "re:" + maindomain + "/yolo.*/"
+ assert _check_and_sanitize_permission_path("re:" + maindomain + "/y.*o(o+)[a-z]*/bo\1y", maindomain + '/path', 'test_permission.main') == "re:" + maindomain + "/y.*o(o+)[a-z]*/bo\1y"
def test_normalize_permission_path_with_bad_regex():
# Relative Regex
with pytest.raises(YunohostError):
- _check_and_normalize_permission_path("re:/yolo.*[1-7]^?/")
+ _check_and_sanitize_permission_path("re:/yolo.*[1-7]^?/", maindomain + '/path', 'test_permission.main')
with pytest.raises(YunohostError):
- _check_and_normalize_permission_path("re:/yolo.*[1-7](]/")
+ _check_and_sanitize_permission_path("re:/yolo.*[1-7](]/", maindomain + '/path', 'test_permission.main')
# Full Regex
with pytest.raises(YunohostError):
- _check_and_normalize_permission_path("re:" + maindomain + "/yolo?+/")
+ _check_and_sanitize_permission_path("re:" + maindomain + "/yolo?+/", maindomain + '/path', 'test_permission.main')
with pytest.raises(YunohostError):
- _check_and_normalize_permission_path("re:" + maindomain + "/yolo[1-9]**/")
+ _check_and_sanitize_permission_path("re:" + maindomain + "/yolo[1-9]**/", maindomain + '/path', 'test_permission.main')
def test_normalize_permission_path_with_unknown_domain():
with pytest.raises(YunohostError):
- _check_and_normalize_permission_path("shouldntexist.tld/hey")
+ _check_and_sanitize_permission_path("shouldntexist.tld/hey", maindomain + '/path', 'test_permission.main')
with pytest.raises(YunohostError):
- _check_and_normalize_permission_path("re:shouldntexist.tld/hey.*")
+ _check_and_sanitize_permission_path("re:shouldntexist.tld/hey.*", maindomain + '/path', 'test_permission.main')
+
+
+def test_normalize_permission_path_conflicting_path():
+ app_install("./tests/apps/register_url_app_ynh",
+ args="domain=%s&path=%s" % (maindomain, "/url/registerapp"), force=True)
+
+ with pytest.raises(YunohostError):
+ _check_and_sanitize_permission_path("/registerapp", maindomain + '/url', 'test_permission.main')
+ with pytest.raises(YunohostError):
+ _check_and_sanitize_permission_path(maindomain + "/url/registerapp", maindomain + '/path', 'test_permission.main')