From: Kevin Day <thekevinday@gmail.com>
Date: Sun, 17 Apr 2022 00:26:18 +0000 (-0500)
Subject: Bugfix: Problems in f_file regarding file mode exposed by unit tests.
X-Git-Tag: 0.5.10~202
X-Git-Url: https://git.kevux.org/?a=commitdiff_plain;h=f450016534870cfbfd8bbf31035bbf45b444ec2a;p=fll

Bugfix: Problems in f_file regarding file mode exposed by unit tests.

The f_file_mode_from_string() function clearly didn't survive multiple refactors.
There are problems clearly the result from mass-refactoring.

Now that the parameter is f_string_static_t rather than an f_string_t, use the ".used" rather than NULL checks to determine end of string.
Failure to do this could result in unexpected behavior.

There are also bugs and mistakes that I do not know how they even got past me.
The comparison checks are missing from some checks!
Add missing '==' comparisons.

Exit as soon as possible when code.used is smaller than it is allowed to be.
Mode strings that start with '+', '-', or '=' of length 1 cannot be valid.
Mode strings that start with '=' should replace across all blocks.

Better detect when the code is incomplete and return an error.

Add missing detection for when mode string is too large for number-based modes.

As per chmod command, replacement digits (not having '+' or '-') result in replacing special bits as well.

Update the documentation to better describe how f_file_mode_t works.
---

diff --git a/level_0/f_file/c/file.c b/level_0/f_file/c/file.c
index 23c160a..24cf181 100644
--- a/level_0/f_file/c/file.c
+++ b/level_0/f_file/c/file.c
@@ -957,7 +957,12 @@ extern "C" {
     f_file_mode_t mode_result = 0;
 
     if (code.string[0] == f_string_ascii_plus_s.string[0] || code.string[0] == f_string_ascii_minus_s.string[0] || code.string[0] == f_string_ascii_equal_s.string[0]) {
-      if (code.string[1] == f_string_ascii_r_s.string[0] || f_string_ascii_w_s.string[0] || f_string_ascii_x_s.string[0] || f_string_ascii_X_s.string[0] || f_string_ascii_s_s.string[0] ||f_string_ascii_t_s.string[0]) {
+
+      if (code.used < 2) {
+        return F_status_set_error(F_syntax);
+      }
+
+      if (code.string[1] == f_string_ascii_r_s.string[0] || code.string[1] == f_string_ascii_w_s.string[0] || code.string[1] == f_string_ascii_x_s.string[0] || code.string[1] == f_string_ascii_X_s.string[0] || code.string[1] == f_string_ascii_s_s.string[0] ||code.string[1] == f_string_ascii_t_s.string[0]) {
         syntax = 1;
       }
       else if (code.string[1] == f_string_ascii_0_s.string[0] || code.string[1] == f_string_ascii_1_s.string[0] || code.string[1] == f_string_ascii_2_s.string[0] || code.string[1] == f_string_ascii_3_s.string[0] || code.string[1] == f_string_ascii_4_s.string[0] || code.string[1] == f_string_ascii_5_s.string[0] || code.string[1] == f_string_ascii_6_s.string[0] || code.string[1] == f_string_ascii_7_s.string[0]) {
@@ -968,6 +973,11 @@ extern "C" {
       }
     }
     else if (code.string[0] == f_string_ascii_u_s.string[0] || code.string[0] == f_string_ascii_g_s.string[0] || code.string[0] == f_string_ascii_o_s.string[0] || code.string[0] == f_string_ascii_a_s.string[0]) {
+
+      if (code.used < 2) {
+        return F_status_set_error(F_syntax);
+      }
+
       syntax = 1;
     }
     else if (code.string[0] == f_string_ascii_0_s.string[0] || code.string[0] == f_string_ascii_1_s.string[0] || code.string[0] == f_string_ascii_2_s.string[0] || code.string[0] == f_string_ascii_3_s.string[0] || code.string[0] == f_string_ascii_4_s.string[0] || code.string[0] == f_string_ascii_5_s.string[0] || code.string[0] == f_string_ascii_6_s.string[0] || code.string[0] == f_string_ascii_7_s.string[0]) {
@@ -980,6 +990,7 @@ extern "C" {
     if (syntax == 1) {
       uint8_t on = 0; // 1 = user, 2 = group, 4 = world/sticky, 7 = all.
       uint8_t how = 0; // 1 = add, 2 = replace, 3 = subtract, 4 = add when no "on", 5 = replace when no "on", and 6 = subtract when no "on".
+      bool incomplete = F_true;
 
       f_file_mode_t mode_mask = 0;
       f_file_mode_t mode_umask = 0;
@@ -1034,7 +1045,7 @@ extern "C" {
         mode_umask |= F_file_mode_t_block_world_d & F_file_mode_t_mask_bit_execute_d;
       }
 
-      for (f_array_length_t i = 0; syntax && code.string[i]; ++i) {
+      for (f_array_length_t i = 0; syntax && i < code.used; ++i) {
 
         if (code.string[i] == f_string_ascii_u_s.string[0]) {
           on |= 1;
@@ -1068,16 +1079,22 @@ extern "C" {
 
             replace_result |= F_file_mode_t_replace_special_d;
 
-            if (mode_mask & F_file_mode_t_block_owner_d) {
-              replace_result |= F_file_mode_t_replace_owner_d;
-            }
+            // When i is 0, then '=' is applied to all blocks.
+            if (i) {
+              if (mode_mask & F_file_mode_t_block_owner_d) {
+                replace_result |= F_file_mode_t_replace_owner_d;
+              }
 
-            if (mode_mask & F_file_mode_t_block_group_d) {
-              replace_result |= F_file_mode_t_replace_group_d;
-            }
+              if (mode_mask & F_file_mode_t_block_group_d) {
+                replace_result |= F_file_mode_t_replace_group_d;
+              }
 
-            if (mode_mask & F_file_mode_t_block_world_d) {
-              replace_result |= F_file_mode_t_replace_world_d;
+              if (mode_mask & F_file_mode_t_block_world_d) {
+                replace_result |= F_file_mode_t_replace_world_d;
+              }
+            }
+            else {
+              replace_result |= F_file_mode_t_replace_owner_d | F_file_mode_t_replace_group_d | F_file_mode_t_replace_world_d;
             }
           }
 
@@ -1088,7 +1105,7 @@ extern "C" {
 
           what = 0;
 
-          for (++i; code.string[i]; ++i) {
+          for (++i; i < code.used; ++i) {
 
             if (code.string[i] == f_string_ascii_r_s.string[0]) {
               what = F_file_mode_t_mask_bit_read_d;
@@ -1126,6 +1143,8 @@ extern "C" {
               }
             }
             else if (code.string[i] == f_string_ascii_comma_s.string[0]) {
+              incomplete = F_false;
+
               if (how > 3) {
                 mode_result -= mode_result & mode_umask;
               }
@@ -1175,6 +1194,7 @@ extern "C" {
 
             if (what) {
               if (how == 1 || how == 2 || how == 4 || how == 5) {
+                incomplete = F_false;
                 mode_result |= what & mode_mask & F_file_mode_t_mask_how_add_d;
 
                 if (mode_result & what & mode_mask & F_file_mode_t_mask_how_subtract_d) {
@@ -1182,6 +1202,7 @@ extern "C" {
                 }
               }
               else if (how == 3 || how == 6) {
+                incomplete = F_false;
                 mode_result |= what & mode_mask & F_file_mode_t_mask_how_subtract_d;
 
                 if (mode_result & what & mode_mask & F_file_mode_t_mask_how_add_d) {
@@ -1192,15 +1213,20 @@ extern "C" {
           } // for
 
           if (how > 3) {
+            incomplete = F_false;
             mode_result -= mode_result & mode_umask;
           }
 
           if (!code.string[i]) break;
         }
         else {
-          syntax = 0;
+          return F_status_set_error(F_syntax);
         }
       } // for
+
+      if (incomplete) {
+        return F_status_set_error(F_syntax);
+      }
     }
     else if (syntax == 2) {
 
@@ -1225,12 +1251,12 @@ extern "C" {
         how = 2;
         i = 1;
 
-        replace_result = F_file_mode_t_replace_standard_d;
+        replace_result = F_file_mode_t_replace_standard_d | F_file_mode_t_replace_special_d;
       }
       else {
         how = 2;
 
-        replace_result = F_file_mode_t_replace_standard_d | F_file_mode_t_replace_directory_d;
+        replace_result = F_file_mode_t_replace_standard_d | F_file_mode_t_replace_special_d;
       }
 
       // Seek past leading '0's.
@@ -1238,19 +1264,24 @@ extern "C" {
         ++i;
       } // while
 
-      if (code.string[i]) {
+      if (i < code.used) {
         f_array_length_t j = 0;
 
-        for (; code.string[i + j] && j < 4; ++j) {
+        // Do not tolerate overly large numbers.
+        if (code.used - i > 4) {
+          return F_status_set_error(F_syntax);
+        }
+
+        for (; i + j < code.used; ++j) {
 
           if (j) {
             mode_result <<= 8;
           }
 
-          if (code.string[i] == f_string_ascii_0_s.string[0]) {
+          if (code.string[i + j] == f_string_ascii_0_s.string[0]) {
             // Already is a zero.
           }
-          else if (code.string[i] == f_string_ascii_1_s.string[0] || code.string[i] == f_string_ascii_2_s.string[0] || code.string[i] == f_string_ascii_3_s.string[0] || code.string[i] == f_string_ascii_4_s.string[0] || code.string[i] == f_string_ascii_5_s.string[0] || code.string[i] == f_string_ascii_6_s.string[0] || code.string[i] == f_string_ascii_7_s.string[0]) {
+          else if (code.string[i + j] == f_string_ascii_1_s.string[0] || code.string[i + j] == f_string_ascii_2_s.string[0] || code.string[i + j] == f_string_ascii_3_s.string[0] || code.string[i + j] == f_string_ascii_4_s.string[0] || code.string[i + j] == f_string_ascii_5_s.string[0] || code.string[i + j] == f_string_ascii_6_s.string[0] || code.string[i + j] == f_string_ascii_7_s.string[0]) {
 
             // This assumes ASCII/UTF-8.
             if (how == 3) {
@@ -1264,6 +1295,7 @@ extern "C" {
 
             // Designate that this is invalid.
             j = 4;
+
             break;
           }
         } // for
@@ -1271,13 +1303,6 @@ extern "C" {
         if (j == 4) {
           syntax = 0;
         }
-        else if (how == 2) {
-
-          // If there are only '0's then the standard and setuid/setgid/sticky bits are to be replaced.
-          if (!mode_result) {
-            replace_result = F_file_mode_t_replace_standard_d | F_file_mode_t_replace_special_d;
-          }
-        }
       }
     }
 
diff --git a/level_0/f_file/c/file.h b/level_0/f_file/c/file.h
index d1e20c3..d6a876b 100644
--- a/level_0/f_file/c/file.h
+++ b/level_0/f_file/c/file.h
@@ -1138,10 +1138,18 @@ extern "C" {
  * @param mode
  *   The determined mode.
  *   This uses bitwise data.
+ *
+ *   See the f_file_mode_t documentation for details.
  * @param replace
  *   The determined modes that are to be replaced, such as: F_file_mode_t_replace_owner_d.
  *   This uses bitwise data.
  *
+ *   The flags F_file_mode_t_replace_* are used to designate which mask bits are to be replaced.
+ *   For example F_file_mode_t_replace_owner_d would designate that the owner bits are to be replaced.
+ *   A value of 0 means that there are no replacements being made.
+ *
+ *   Replacements replace the entire existing mode values where as "add" and "subtract" add or subtract modes, respectively, to the existing mode values.
+ *
  * @return
  *   F_none on success.
  *
@@ -1149,8 +1157,6 @@ extern "C" {
  *   F_syntax (with error bit) if the string fails to follow the syntax rules.
  *
  *   The parameters how, mode_normal, and mode_executable are all set to 0 on error.
- *
- * @see private_f_file_mode_determine()
  */
 #ifndef _di_f_file_mode_from_string_
   extern f_status_t f_file_mode_from_string(const f_string_static_t code, const mode_t umask, f_file_mode_t * const mode, uint8_t * const replace);
diff --git a/level_0/f_file/c/file/common.h b/level_0/f_file/c/file/common.h
index 4554b4a..3ea5832 100644
--- a/level_0/f_file/c/file/common.h
+++ b/level_0/f_file/c/file/common.h
@@ -333,7 +333,17 @@ extern "C" {
  *
  * The second type is f_file_mode_t, which utilizes 8-bit types with the following structure:
  *
- *   There should only be a single bit for each 'r', 'w', 'x', and 'X' bit (as well as 'S', 's', and 't'):
+ *   The f_file_mode_t structure is a 32-bit unsigned integer designed to directly represent the special, owner, group, and world mode bits along with their respective read, write, and execute bits (setuid, setgid, and sticky for the special bits).
+ *
+ *   Each bit is structured in 8-bit blocks as follows:
+ *     [ special ][ owner ][ group ][ world ]
+ *
+ *   Each block, from left to right, has 4 bits representing "subtract" followed by 4 bits representing "add".
+ *   As an exceptional case, the first bit (left most bit) for the "special" block is not used and expected to be 0.
+ *
+ *   Each of these 4-bits, respectively, represents "special", "read", "write", and "execute".
+ *
+ *   Each bit maps to some character 'r', 'w', 'x', and 'X' bit (as well as 'S', 's', and 't' for each "special"):
  *     'r' = read bit.
  *     'w' = write bit.
  *     'x' = execute bit.
@@ -342,6 +352,23 @@ extern "C" {
  *     's' = set group bit (setgid).
  *     't' = sticky bit.
  *
+ *   There exists both an "add" and a "subtract" block so that both operations can be passed.
+ *   Such as "+r,-w" meaning add read and subtract write.
+ *
+ *   For replacements, additional bits and masks are provided which are intended to be used in a separate variable given that only 8-bits are needed for replacements.
+ *
+ *   The replacements are, also from left to write, are broken up into the following bits of a single byte:
+ *     [ unused ] [ unused ] [ unused ] [ directory ] [ special ] [ owner ] [ group ] [ world ]
+ *
+ *   The directory bit is a special case bit used to declare that these bits are in respect to a directory.
+ *   Directory has special representation when it comes to the execute bit, namely when the "execute only" mode is being applied.
+ *   The directory bit may be applied even when there are no replacements.
+ *   This can be used to designate that the "add" or "subtract" is being applied to a directory.
+ *
+ *   When, say the "owner" bit is set in this replacement, this means that the owner bit is to be replaced before performing any add/subtract operations.
+ *   When using the replacement variable, the "subtract" is considered a no-op and is ignored.
+ *   Use only the "add" bits in conjuction with respective "replace" bits.
+ *
  * The file mode macros with "f_file_mode_" prefix (has no "_t") refer to the first type (mode_t).
  * The file mode macros with "f_file_mode_t" prefix refer to the second type (f_file_mode_t).
  */