[libcalamares] Split out file-mode parsing to a separate function

src/libcalamares/CMakeLists.txt

@@ -236,7 +236,7 @@ if(KPMcore_FOUND)
     calamares_add_test(libcalamarespartitionkpmtest SOURCES partition/KPMTests.cpp LIBRARIES calamares::kpmcore)
 endif()
 
-calamares_add_test(libcalamaresutilstest SOURCES utils/Tests.cpp utils/Runner.cpp)
+calamares_add_test(libcalamaresutilstest SOURCES utils/Tests.cpp utils/Permissions.cpp utils/Runner.cpp)
 
 calamares_add_test(libcalamaresutilspathstest SOURCES utils/TestPaths.cpp)
 

src/libcalamares/utils/Permissions.cpp

@@ -50,9 +50,8 @@ Permissions::parsePermissions( QString const& p )
         return;
     }
 
-    bool ok;
+    const auto octal = parseFileMode( segments[ 2 ] );
-    int octal = segments[ 2 ].toInt( &ok, 8 );
+    if ( octal <= 0 )
-    if ( !ok || octal == 0 )
     {
         m_valid = false;
         return;
@@ -120,4 +119,24 @@ Permissions::apply( const QString& path, const Calamares::Permissions& p )
     return r;
 }
 
+int
+parseFileMode( const QString& mode )
+{
+    bool ok;
+    int octal = mode.toInt( &ok, 8 );
+    if ( !ok )
+    {
+        return -1;
+    }
+    if ( 0777 < octal )
+    {
+        return -1;
+    }
+    if ( octal < 0 )
+    {
+        return -1;
+    }
+    return octal;
+}
+
 }  // namespace Calamares

src/libcalamares/utils/Permissions.h

@@ -16,8 +16,10 @@ namespace Calamares
 {
 
 /**
- * @brief The Permissions class takes a QString @p in the form of
+ * @brief Represents a <user>:<group>:<file-mode>
- * <user>:<group>:<permissions>, checks it for validity, and makes the three
+ *
+ * The Permissions class takes a QString @p in the form of
+ * <user>:<group>:<file-mode>, checks it for validity, and makes the three
  * components available indivdually.
  */
 class DLLEXPORT Permissions
@@ -27,8 +29,9 @@ public:
     /** @brief Constructor
      *
      * Splits the string @p at the colon (":") into separate elements for
-     * <user>, <group>, and <value> (permissions), where <value> is interpreted
+     * <user>, <group>, and <file-mode> (permissions), where <file-mode> is any
-     * as an **octal** integer. That is, "root:wheel:755" will give
+     * value that can be parsed by parseFileMode() . One valid form
+     * is an **octal** integer. That is, "root:wheel:755" will give
      * you an integer value of four-hundred-ninety-three (493),
      * corresponding to the UNIX file permissions rwxr-xr-x,
      * as one would expect from chmod and other command-line utilities.
@@ -90,6 +93,17 @@ private:
     bool m_valid;
 };
 
+/**
+ * @brief Parses a file-mode and returns it as an integer
+ *
+ * Returns -1 on error.
+ *
+ * Valid forms of @p mode are:
+ * - octal representation, with an optional leading 0 and at most three
+ *   octal digits (e.g. 0755 or 644)
+ */
+int parseFileMode( const QString& mode );
+
 }  // namespace Calamares
 
 #endif  // LIBCALAMARES_PERMISSIONS_H

src/libcalamares/utils/Tests.cpp

@@ -63,6 +63,7 @@ private Q_SLOTS:
 
     /** @section Test that all the UMask objects work correctly. */
     void testUmask();
+    void testPermissions();
 
     /** @section Tests the entropy functions. */
     void testEntropy();
@@ -571,6 +572,24 @@ LibCalamaresTests::testUmask()
     QCOMPARE( Calamares::setUMask( m ), mode_t( 022 ) );
 }
 
+void
+LibCalamaresTests::testPermissions()
+{
+    for ( int i = 0; i <= 0777; ++i )
+    {
+        const QString repr = QString::number( i, 8 );
+        QCOMPARE( Calamares::parseFileMode( repr ), i );
+        QCOMPARE( Calamares::parseFileMode( QChar( '0' ) + repr ), i );
+        QCOMPARE( Calamares::parseFileMode( QStringLiteral( "  %1\n" ).arg( repr ) ), i );
+    }
+
+    // Failures
+    QCOMPARE( Calamares::parseFileMode( QStringLiteral( "1024" ) ), -1 );
+    QCOMPARE( Calamares::parseFileMode( QStringLiteral( "rwxr-----" ) ), -1 );
+    QCOMPARE( Calamares::parseFileMode( QStringLiteral( "o644" ) ), -1 );
+    QCOMPARE( Calamares::parseFileMode( QStringLiteral( "O_WRONLY" ) ), -1 );
+}
+
 void
 LibCalamaresTests::testEntropy()
 {