From 30f58de800508c3e3c9580e759e9c472dcbec15b Mon Sep 17 00:00:00 2001
From: Xavier Defrang <xavier.defrang@gmail.com>
Date: Tue, 11 Jan 2022 17:15:21 +0100
Subject: [PATCH] LibCore: Restore support for multiple symbolic classes

Reverts recent change introduced to support implicit symbolic permission
which broke the parser when multiple classes are specified.
The state machine must assume it's dealing with classes until an
operation character is consumed.
---
 .../TestLibCoreFilePermissionsMask.cpp        | 21 +++++++
 .../Libraries/LibCore/FilePermissionsMask.cpp | 57 ++++++++-----------
 2 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/Tests/LibCore/TestLibCoreFilePermissionsMask.cpp b/Tests/LibCore/TestLibCoreFilePermissionsMask.cpp
index 544bd750a0..20f1e9899e 100644
--- a/Tests/LibCore/TestLibCoreFilePermissionsMask.cpp
+++ b/Tests/LibCore/TestLibCoreFilePermissionsMask.cpp
@@ -44,6 +44,13 @@ TEST_CASE(file_permission_mask_from_symbolic_notation)
     EXPECT_EQ(mask.value().apply(0), 0555);
     EXPECT_EQ(mask.value().apply(0664), 0555);
 
+    mask = Core::FilePermissionsMask::from_symbolic_notation("ugo=rx"sv);
+    EXPECT(!mask.is_error());
+    EXPECT_EQ(mask.value().clear_mask(), 0777);
+    EXPECT_EQ(mask.value().write_mask(), 0555);
+    EXPECT_EQ(mask.value().apply(0), 0555);
+    EXPECT_EQ(mask.value().apply(0664), 0555);
+
     mask = Core::FilePermissionsMask::from_symbolic_notation("u+rw,g=rx,o-rwx"sv);
     EXPECT(!mask.is_error());
     EXPECT_EQ(mask.value().clear_mask(), 0077);
@@ -51,6 +58,20 @@ TEST_CASE(file_permission_mask_from_symbolic_notation)
     EXPECT_EQ(mask.value().apply(0), 0650);
     EXPECT_EQ(mask.value().apply(0177), 0750);
 
+    mask = Core::FilePermissionsMask::from_symbolic_notation("+r"sv);
+    EXPECT(!mask.is_error());
+    EXPECT_EQ(mask.value().clear_mask(), 0);
+    EXPECT_EQ(mask.value().write_mask(), 0444);
+    EXPECT_EQ(mask.value().apply(0), 0444);
+    EXPECT_EQ(mask.value().apply(0123), 0567);
+
+    mask = Core::FilePermissionsMask::from_symbolic_notation("=rx"sv);
+    EXPECT(!mask.is_error());
+    EXPECT_EQ(mask.value().clear_mask(), 0777);
+    EXPECT_EQ(mask.value().write_mask(), 0555);
+    EXPECT_EQ(mask.value().apply(0), 0555);
+    EXPECT_EQ(mask.value().apply(0664), 0555);
+
     mask = Core::FilePermissionsMask::from_symbolic_notation("z+rw"sv);
     EXPECT(mask.is_error());
 
diff --git a/Userland/Libraries/LibCore/FilePermissionsMask.cpp b/Userland/Libraries/LibCore/FilePermissionsMask.cpp
index 92f632e72f..fe759725a5 100644
--- a/Userland/Libraries/LibCore/FilePermissionsMask.cpp
+++ b/Userland/Libraries/LibCore/FilePermissionsMask.cpp
@@ -14,14 +14,14 @@ namespace Core {
 
 enum State {
     Classes,
-    Operation,
     Mode
 };
 
 enum ClassFlag {
     Other = 1,
     Group = 2,
-    User = 4
+    User = 4,
+    All = 7
 };
 
 enum Operation {
@@ -56,43 +56,32 @@ ErrorOr<FilePermissionsMask> FilePermissionsMask::from_symbolic_notation(StringV
     for (auto ch : string) {
         switch (state) {
         case State::Classes: {
-            // one or more [ugoa] terminated by one operator [+-=]
-            if (ch == 'u') {
+            // zero or more [ugoa] terminated by one operator [+-=]
+            if (ch == 'u')
                 classes |= ClassFlag::User;
-                state = State::Operation;
-                break;
-            } else if (ch == 'g') {
+            else if (ch == 'g')
                 classes |= ClassFlag::Group;
-                state = State::Operation;
-                break;
-            } else if (ch == 'o') {
+            else if (ch == 'o')
                 classes |= ClassFlag::Other;
-                state = State::Operation;
-                break;
-            } else if (ch == 'a') {
-                classes = ClassFlag::User | ClassFlag::Group | ClassFlag::Other;
-                state = State::Operation;
-                break;
+            else if (ch == 'a')
+                classes = ClassFlag::All;
+            else {
+                if (ch == '+')
+                    operation = Operation::Add;
+                else if (ch == '-')
+                    operation = Operation::Remove;
+                else if (ch == '=')
+                    operation = Operation::Assign;
+                else
+                    return Error::from_string_literal("invalid operation: expected '+', '-' or '='"sv);
+
+                // if an operation was specified without a class, assume all
+                if (classes == 0)
+                    classes = ClassFlag::All;
+
+                state = State::Mode;
             }
-        }
-            [[fallthrough]];
-        case State::Operation: {
-            if (ch == '+')
-                operation = Operation::Add;
-            else if (ch == '-')
-                operation = Operation::Remove;
-            else if (ch == '=')
-                operation = Operation::Assign;
-            else if (classes == 0)
-                return Error::from_string_literal("invalid access class: expected 'u', 'g', 'o' or 'a' "sv);
-            else
-                return Error::from_string_literal("invalid operation: expected '+', '-' or '='"sv);
 
-            // if an operation was specified without a class, assume all
-            if (classes == 0)
-                classes = ClassFlag::User | ClassFlag::Group | ClassFlag::Other;
-
-            state = State::Mode;
             break;
         }