diff --git a/fdroidserver/common.py b/fdroidserver/common.py index ad056937..fbfc8ddb 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -75,6 +75,10 @@ VERCODE_OPERATION_RE = re.compile(r'^([ 0-9/*+-]|%c)+$') CERT_PATH_REGEX = re.compile(r'^META-INF/.*\.(DSA|EC|RSA)$') APK_NAME_REGEX = re.compile(r'^([a-zA-Z][\w.]*)_(-?[0-9]+)_?([0-9a-f]{7})?\.apk') STANDARD_FILE_NAME_REGEX = re.compile(r'^(\w[\w.]*)_(-?[0-9]+)\.\w+') +FDROID_PACKAGE_NAME_REGEX = re.compile(r'''^[a-f0-9]+$''', re.IGNORECASE) +STRICT_APPLICATION_ID_REGEX = re.compile(r'''(?:^[a-z_]+(?:\d*[a-zA-Z_]*)*)(?:\.[a-z_]+(?:\d*[a-zA-Z_]*)*)*$''') +VALID_APPLICATION_ID_REGEX = re.compile(r'''(?:^[a-z_]+(?:\d*[a-zA-Z_]*)*)(?:\.[a-z_]+(?:\d*[a-zA-Z_]*)*)*$''', + re.IGNORECASE) MAX_VERSION_CODE = 0x7fffffff # Java's Integer.MAX_VALUE (2147483647) @@ -1516,8 +1520,12 @@ def parse_androidmanifests(paths, app): if max_version is None: max_version = "Unknown" - if max_package and not is_valid_java_package_name(max_package): - raise FDroidException(_("Invalid package name {0}").format(max_package)) + if max_package: + msg = _("Invalid package name {0}").format(max_package) + if not is_valid_package_name(max_package): + raise FDroidException(msg) + elif not is_strict_application_id(max_package): + logging.warning(msg) return (max_version, max_vercode, max_package) @@ -1530,12 +1538,25 @@ def is_valid_package_name(name): files use the SHA-256 sum. """ - return re.match("^([a-f0-9]+|[A-Za-z_][A-Za-z_0-9.]+)$", name) + return VALID_APPLICATION_ID_REGEX.match(name) is not None \ + or FDROID_PACKAGE_NAME_REGEX.match(name) is not None -def is_valid_java_package_name(name): - """Check whether name is a valid Java package name aka Application ID""" - return re.match("^[A-Za-z_][A-Za-z_0-9.]+$", name) +def is_strict_application_id(name): + """Check whether name is a valid Android Application ID + + The Android ApplicationID is basically a Java Package Name, but + with more restrictive naming rules: + + * It must have at least two segments (one or more dots). + * Each segment must start with a letter. + * All characters must be alphanumeric or an underscore [a-zA-Z0-9_]. + + https://developer.android.com/studio/build/application-id + + """ + return STRICT_APPLICATION_ID_REGEX.match(name) is not None \ + and '.' in name def getsrclib(spec, srclib_dir, subdir=None, basepath=False, diff --git a/fdroidserver/lint.py b/fdroidserver/lint.py index 6a662010..bfa380e7 100644 --- a/fdroidserver/lint.py +++ b/fdroidserver/lint.py @@ -488,7 +488,13 @@ def check_for_unsupported_metadata_files(basedir=""): if not exists: print(_('"%s/" has no matching metadata file!') % f) return_value = True - elif not os.path.splitext(f)[1][1:] in formats: + elif os.path.splitext(f)[1][1:] in formats: + packageName = os.path.splitext(os.path.basename(f))[0] + if not common.is_valid_package_name(packageName): + print('"' + packageName + '" is an invalid package name!\n' + + 'https://developer.android.com/studio/build/application-id') + return_value = True + else: print('"' + f.replace(basedir, '') + '" is not a supported file format: (' + ','.join(formats) + ')') return_value = True diff --git a/fdroidserver/update.py b/fdroidserver/update.py index 740ff6e2..60e71539 100644 --- a/fdroidserver/update.py +++ b/fdroidserver/update.py @@ -1067,9 +1067,12 @@ def scan_apk(apk_file): else: scan_apk_aapt(apk, apk_file) - if not common.is_valid_java_package_name(apk['packageName']): + if not common.is_valid_package_name(apk['packageName']): raise BuildException(_("{appid} from {path} is not a valid Java Package Name!") .format(appid=apk['packageName'], path=apk_file)) + elif not common.is_strict_application_id(apk['packageName']): + logging.warning(_("{appid} from {path} is not a valid Java Package Name!") + .format(appid=apk['packageName'], path=apk_file)) # Get the signature, or rather the signing key fingerprints logging.debug('Getting signature of {0}'.format(os.path.basename(apk_file))) diff --git a/tests/common.TestCase b/tests/common.TestCase index ef614a56..9e2076d0 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -159,8 +159,10 @@ class CommonTest(unittest.TestCase): "debuggable APK state was not properly parsed!") def test_is_valid_package_name(self): - for name in ["org.fdroid.fdroid", + for name in ["cafebabe", + "org.fdroid.fdroid", "org.f_droid.fdr0ID", + "SpeedoMeterApp.main", "05041684efd9b16c2888b1eddbadd0359f655f311b89bdd1737f560a10d20fb8"]: self.assertTrue(fdroidserver.common.is_valid_package_name(name), "{0} should be a valid package name".format(name)) @@ -171,18 +173,22 @@ class CommonTest(unittest.TestCase): self.assertFalse(fdroidserver.common.is_valid_package_name(name), "{0} should not be a valid package name".format(name)) - def test_is_valid_java_package_name(self): + def test_is_strict_application_id(self): + """see also tests/valid-package-names/""" for name in ["org.fdroid.fdroid", "org.f_droid.fdr0ID"]: - self.assertTrue(fdroidserver.common.is_valid_java_package_name(name), - "{0} should be a valid package name".format(name)) + self.assertTrue(fdroidserver.common.is_strict_application_id(name), + "{0} should be a strict application id".format(name)) for name in ["0rg.fdroid.fdroid", ".f_droid.fdr0ID", + "oneword", + "cafebabe", + "SpeedoMeterApp.main", "org.fdroid/fdroid", "/org.fdroid.fdroid", "05041684efd9b16c2888b1eddbadd0359f655f311b89bdd1737f560a10d20fb8"]: - self.assertFalse(fdroidserver.common.is_valid_java_package_name(name), - "{0} should not be a valid package name".format(name)) + self.assertFalse(fdroidserver.common.is_strict_application_id(name), + "{0} should not be a strict application id".format(name)) def test_prepare_sources(self): testint = 99999999 diff --git a/tests/valid-package-names/RandomPackageNames.java b/tests/valid-package-names/RandomPackageNames.java new file mode 100644 index 00000000..82d77bbe --- /dev/null +++ b/tests/valid-package-names/RandomPackageNames.java @@ -0,0 +1,237 @@ + +import java.io.BufferedReader; +import java.io.File; +import java.io.FileOutputStream; +import java.io.Writer; +import java.io.InputStreamReader; +import java.io.IOException; +import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.UnsupportedEncodingException; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.IntBuffer; +import java.util.Random; + +// apt-get install libcommons-lang3-java +//import org.apache.commons.lang3.RandomStringUtils; + +public class RandomPackageNames { + + private static Writer validWriter; + private static Writer invalidWriter; + + private static final String[] py = { + "python3", "-c", + "import sys,re\n" + + "m = re.search(r'''" + // + "^(?:[a-z_]+(?:\\d*[a-zA-Z_]*)*)(?:\\.[a-z_]+(?:\\d*[a-zA-Z_]*)*)*$" + + "^[a-z_]+(?:\\d*[a-zA-Z_]*)(?:\\.[a-z_]+(?:\\d*[a-zA-Z_]*)*)*$" + + "''', sys.stdin.read())\n" + + "if m is not None:\n" + + " with open('/tmp/foo', 'w') as fp:\n" + + " fp.write(m.group() + '\\n')\n" + + "sys.exit(m is None)" + }; + + public static boolean checkAgainstPython(String packageName) + throws IOException, InterruptedException { + + ProcessBuilder pb = new ProcessBuilder(py); + Process process = pb.start(); + OutputStream output = process.getOutputStream(); + output.write(packageName.getBytes()); + output.write("\n".getBytes()); + output.flush(); + output.close(); + + int exitVal = process.waitFor(); + return exitVal == 0; + } + + private static boolean isValidJavaIdentifier(String packageName) { + if (packageName.length() == 0 || !Character.isJavaIdentifierStart(packageName.charAt(0))) { + //System.out.println("invalid first char: '" + packageName + "'"); + return false; + } + for (int codePoint : packageName.codePoints().toArray()) { + if (codePoint != 46 && !Character.isJavaIdentifierPart(codePoint)) { + //System.out.println("invalid char: '" + // + new StringBuilder().appendCodePoint(codePoint).toString() + "' " + // + codePoint); + return false; + } + } + return true; + } + + private static void write(String packageName) throws IOException { + if (isValidJavaIdentifier(packageName)) { + validWriter.write(packageName); + validWriter.write("\n"); + } else { + invalidWriter.write(packageName); + invalidWriter.write("\n"); + } + } + + private static void compare(String packageName) + throws IOException, InterruptedException { + boolean python = checkAgainstPython(packageName); + boolean java = isValidJavaIdentifier(packageName); + if (python && !java) { + System.out.println("MISMATCH: '" + packageName + "' " + + (python ? "py:✔" : "py:☹") + " " + + (java ? "ja:✔" : "ja:☹") + " "); + } + } + + public static void main (String[] args) + throws IOException, InterruptedException, UnsupportedEncodingException { + int[] data; + byte[] bytes; + ByteBuffer byteBuffer; + Random random = new Random(); + + validWriter = new OutputStreamWriter(new FileOutputStream("valid.txt"), "UTF-8"); + invalidWriter = new OutputStreamWriter(new FileOutputStream("invalid.txt"), "UTF-8"); + + //System.out.print("."); + + char[] validFirstLetters = new char[27]; + validFirstLetters[0] = 95; // _ + for (int i = 1; i < 27; i++) { + validFirstLetters[i] = (char) (i + 96); + } + + char[] validLetters = new char[64]; + int j = 0; + for (char c = 32; c < 123; c++) { + if ((c == 46) || (c > 47 && c < 58) || (c > 64 && c < 91) || (c > 96)) { + validLetters[j] = c; + j++; + } + } + + for (File f : new File("/home/hans/code/fdroid/fdroiddata/metadata").listFiles()) { + String name = f.getName(); + if (name.endsWith(".yml") || name.endsWith(".txt")) { + compare(name.substring(0, name.length() - 4)); + } + } + compare("SpeedoMeterApp.main"); + compare("uk.co.turtle-player"); + compare("oVPb"); + compare(" _LS"); + compare("r.vq"); + compare("r.vQ"); + compare("ra.vQ"); + compare("s.vQ"); + compare("r.tQ"); + compare("r.vR"); + compare("any.any"); + compare("org.fdroid.fdroid"); + compare("me.unfollowers.droid"); + compare("me_.unfollowers.droid"); + compare("me._unfollowers.droid"); + compare("me.unfo11llowers.droid"); + compare("me11.unfollowers.droid"); + compare("m11e.unfollowers.droid"); + compare("1me.unfollowers.droid"); + compare("me.unfollowers23.droid"); + compare("me.unfollowers.droid23d"); + compare("me.unfollowers_.droid"); + compare("me.unfollowers._droid"); + compare("me.unfollowers_._droid"); + compare("me.unfollowers.droid_"); + compare("me.unfollowers.droid32"); + compare("me.unfollowers.droid/"); + compare("me:.unfollowers.droid"); + compare(":me.unfollowers.droid"); + compare("me.unfollowers.dro;id"); + compare("me.unfollowe^rs.droid"); + compare("me.unfollowers.droid."); + compare("me.unfollowers..droid"); + compare("me.unfollowers.droid._"); + compare("me.unfollowers.11212"); + compare("me.1.unfollowers.11212"); + compare("me..unfollowers.11212"); + compare("abc"); + compare("abc."); + compare(".abc"); + + for (int i = 0; i < 300000; i++) { + String packageName; + + int count = random.nextInt(10) + 1; + byte valid = (byte) random.ints(97, 122).limit(1).toArray()[0]; + + // only valid + data = random.ints(46, 122) + .limit(count) + .filter(c -> (c == 46) || (c > 47 && c < 58) || (c > 64 && c < 91) || (c > 96)) + .toArray(); + byteBuffer = ByteBuffer.allocate(data.length); + for (int value : data) { + byteBuffer.put((byte)value); + } + if (data.length > 0) { + bytes = byteBuffer.array(); + bytes[0] = valid; + packageName = new String(byteBuffer.array(), "UTF-8"); + //System.out.println(packageName + ": " + isValidJavaIdentifier(packageName)); + compare(packageName); + write(packageName); + } + + // full US-ASCII + data = random.ints(32, 126).limit(count).toArray(); + byteBuffer = ByteBuffer.allocate(data.length); + for (int value : data) { + byteBuffer.put((byte)value); + } + bytes = byteBuffer.array(); + packageName = new String(bytes, "UTF-8"); + //System.out.println(packageName + ": " + isValidJavaIdentifier(packageName)); + compare(packageName); + write(packageName); + + // full US-ASCII with valid first letter + data = random.ints(32, 127).limit(count).toArray(); + byteBuffer = ByteBuffer.allocate(data.length * 4); + byteBuffer.asIntBuffer().put(data); + bytes = byteBuffer.array(); + bytes[0] = valid; + packageName = new String(bytes, "UTF-8"); + //System.out.println(packageName + ": " + isValidJavaIdentifier(packageName)); + compare(packageName); + write(packageName); + + // full unicode + data = random.ints(32, 0xFFFD).limit(count).toArray(); + byteBuffer = ByteBuffer.allocate(data.length * 4); + byteBuffer.asIntBuffer().put(data); + packageName = new String(byteBuffer.array(), "UTF-32"); + //System.out.println(packageName + ": " + isValidJavaIdentifier(packageName)); + compare(packageName); + write(packageName); + + // full unicode with valid first letter + data = random.ints(32, 0xFFFD).limit(count).toArray(); + byteBuffer = ByteBuffer.allocate(data.length * 4); + byteBuffer.asIntBuffer().put(data); + bytes = byteBuffer.array(); + bytes[0] = 0; + bytes[1] = 0; + bytes[2] = 0; + bytes[3] = 120; + packageName = new String(bytes, "UTF-32"); + //System.out.println(packageName + ": " + isValidJavaIdentifier(packageName)); + compare(packageName); + write(packageName); + } + + validWriter.close(); + invalidWriter.close(); + } +} diff --git a/tests/valid-package-names/random-package-names b/tests/valid-package-names/random-package-names new file mode 100755 index 00000000..bd48d43e --- /dev/null +++ b/tests/valid-package-names/random-package-names @@ -0,0 +1,10 @@ +#!/bin/sh + +set -e +set -x + +export CLASSPATH=/usr/share/java/commons-lang3.jar:. + +cd $(dirname $0) +javac -classpath $CLASSPATH RandomPackageNames.java +java -classpath $CLASSPATH RandomPackageNames diff --git a/tests/valid-package-names/test.py b/tests/valid-package-names/test.py new file mode 100755 index 00000000..eb9f95e3 --- /dev/null +++ b/tests/valid-package-names/test.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python3 + +import re + + +def test(packageName): + m = ANDROID_APPLICATION_ID_REGEX.match(packageName.strip()) + return m is not None + + +ANDROID_APPLICATION_ID_REGEX = re.compile(r'''(?:^[a-z_]+(?:\d*[a-zA-Z_]*)*)(?:\.[a-z_]+(?:\d*[a-zA-Z_]*)*)*$''') +valid = 0 +invalid = 0 + +test('org.fdroid.fdroid') +with open('valid.txt', encoding="utf-8") as fp: + for packageName in fp: + packageName = packageName.strip() + if not test(packageName): + valid += 1 + # print('should be valid:', packageName) + +with open('invalid.txt', encoding="utf-8") as fp: + for packageName in fp: + packageName = packageName.strip() + if test(packageName): + invalid += 1 + print('should be not valid: "' + packageName + '"') + + +print(valid, 'Java thinks is valid, but the Android regex does not') +print(invalid, 'invalid mistakes')