[X2Go-Commits] [x2goserver] 15/30: x2goserver/lib/x2goupdateoptionsstring: refactor remove_option subprocedure into a more general transform_intermediate one and add code for the modification/addition case.

git-admin at x2go.org git-admin at x2go.org
Thu Dec 13 11:22:47 CET 2018


This is an automated email from the git hooks/post-receive script.

x2go pushed a commit to branch master
in repository x2goserver.

commit 0bb83397360a24fa35f6586629395126506132cb
Author: Mihai Moldovan <ionic at ionic.de>
Date:   Sun Dec 9 11:23:57 2018 +0100

    x2goserver/lib/x2goupdateoptionsstring: refactor remove_option subprocedure into a more general transform_intermediate one and add code for the modification/addition case.
---
 debian/changelog                       |   3 +
 x2goserver/lib/x2goupdateoptionsstring | 290 +++++++++++++++++++++++++++------
 2 files changed, 247 insertions(+), 46 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index cdcb15d..82a67e2 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -38,6 +38,9 @@ x2goserver (4.1.0.4-0x2go1) UNRELEASED; urgency=medium
     - x2goserver/lib/x2goupdateoptionsstring: update documentation to include
       the notion of the +key[=value] syntax, which makes sense to use if key
       starts with a dash and would be ambiguously interpreted as a removal.
+    - x2goserver/lib/x2goupdateoptionsstring: refactor remove_option
+      subprocedure into a more general transform_intermediate one and add code
+      for the modification/addition case.
   * debian/control:
     + Build-depend upon lsb-release for distro version detection.
   * debian/x2goserver.manpages:
diff --git a/x2goserver/lib/x2goupdateoptionsstring b/x2goserver/lib/x2goupdateoptionsstring
index c51fd73..f49d403 100755
--- a/x2goserver/lib/x2goupdateoptionsstring
+++ b/x2goserver/lib/x2goupdateoptionsstring
@@ -287,62 +287,101 @@ sub intermediate_to_string {
   return $ret;
 }
 
-# Helper for a grep operation on the intermediate options array.
+# Helper function that checks for errors in options passed as filter
+# parameters (which, in turn, are the parameter this function expects).
 #
-# Takes the option to remove, the current element and amount of elements left
-# in the array as arguments and returns true if the element is not to be
-# removed, false otherwise.
-sub filter_option_remove {
+# Returns true if all checks passed, false otherwise.
+sub sanitize_input_filter {
   my $ret = 1;
-  my $error_detected = 0;
-  my $to_remove = shift;
+
+  my $work_option_key = shift;
+  my $work_option_value = shift;
   my $cur_option = shift;
   my $elems_left = shift;
 
-  if (!((defined ($to_remove)) && (defined ($cur_option)) && (defined ($elems_left)))) {
-    # Undefined values are an error in this context, but erroring out in a
-    # comparison function is... weird, so let's treat such errors as "don't
-    # modify the array".
-    print {*STDERR} "Invalid options passed to removal filter, keeping entry.\n";
-    $error_detected = 1;
+  if (!((defined ($work_option_key)) && (defined ($cur_option)) && (defined ($elems_left)))) {
+    print {*STDERR} "Invalid options passed to filter, keeping entry.\n";
+    $ret = 0;
   }
 
-  if (!($error_detected)) {
+  if ($ret) {
     if ('HASH' ne ref ($cur_option)) {
-      print {*STDERR} "Option passed to removal filter is not a hash reference, keeping entry.\n";
-      $error_detected = 1;
+      print {*STDERR} "Option passed to filter is not a hash reference, keeping entry.\n";
+      $ret = 0;
     }
   }
 
-  if (!($error_detected)) {
+  if ($ret) {
     if (1 < scalar (keys (%{$cur_option}))) {
-      print {*STDERR} "Option passed to removal filter has more than one entry in hash, keeping entry.\n";
-      $error_detected = 1;
+      print {*STDERR} "Option passed to filter has more than one entry in hash, keeping entry.\n";
+      $ret = 0;
     }
   }
 
-  my $to_remove_key = undef;
-  my $to_remove_value = undef;
-  my @to_remove_kv = split (/=/smx, $to_remove, 2);
+  return $ret;
+}
 
-  if (!($error_detected)) {
-    if (2 < scalar (@to_remove_kv)) {
-      print {*STDERR} "Option-to-be-removed string in removal filter has three or more components, this is a bug in $PROGRAM_NAME. Keeping entry.\n";
+# Helper function that splits up the working option into a key and a value.
+#
+# Expects the working option as its only parameter.
+#
+# Returns a reference to an array with two entries - the key and the value.
+# Caveat: the key cannot be undef (it's set to the empty string if it would
+# be), but the value can be undef.
+#
+# In case of errors, returns a reference to undef.
+sub sanitize_workoption_filter {
+  my $ret = undef;
+  my $error_detected = 0;
+
+  my $working_option = shift;
+
+  if (defined ($working_option)) {
+    my $work_key = undef;
+    my $work_value = undef;
+    my @work_kv = split (/=/smx, $working_option, 2);
+
+    if (2 < scalar (@work_kv)) {
+      print {*STDERR} "Option-to-be-acted-upon string in filter has three or more components, this is a bug in $PROGRAM_NAME. Returning error.\n";
       $error_detected = 1;
     }
-  }
 
-  if (!($error_detected) && (0 < $elems_left)) {
-    $to_remove_key = shift (@to_remove_kv);
+    if (!($error_detected)) {
+      $work_key = shift (@work_kv);
+
+      # Key can be undef if splitting failed, e.g., due to an empty input
+      # string. We don't consider this an error, so reset the key to an empty
+      # string.
+      if (!(defined ($work_key))) {
+        $work_key = q{};
+      }
 
-    # Key can be undef if splitting failed, e.g., due to an empty input string.
-    # We don't consider this an error, so reset the key to an empty string.
-    if (!(defined ($to_remove_key))) {
-      $to_remove_key = q{};
+      $work_value = shift (@work_kv);
+
+      $ret = [ $work_key, $work_value ];
     }
+  }
 
-    $to_remove_value = shift (@to_remove_kv);
+  return $ret;
+}
 
+# Helper for a grep operation on the intermediate options array.
+#
+# Takes the option-to-remove's key and value, the current element and amount
+# of elements left in the array as arguments and returns true if the element is
+# not to be removed, false otherwise.
+sub filter_option_remove {
+  my $ret = 1;
+  my $skip = 0;
+  my $to_remove_key = shift;
+  my $to_remove_value = shift;
+  my $cur_option = shift;
+  my $elems_left = shift;
+
+  $skip = ((!(sanitize_input_filter ($to_remove_key, $to_remove_value, $cur_option, $elems_left)))
+           || (0 == $elems_left));
+
+  if (!($skip)) {
     my $option_key = q{};
     my $option_value = undef;
 
@@ -370,23 +409,111 @@ sub filter_option_remove {
   return $ret;
 }
 
-# Removes an entry from the intermediate options array.
+# Helper for a grep operation on the intermediate options array.
+#
+# Takes the option-to-find's key and value, the current element and amount of
+# elements left in the array as arguments and returns true if the element has
+# the same as the option we search for, false otherwise.
+sub filter_find_key {
+  my $ret = 0;
+  my $skip = 0;
+  my $needle_key = shift;
+  my $needle_value = shift;
+  my $cur_option = shift;
+  my $elems_left = shift;
+
+  $skip = ((!(sanitize_input_filter ($needle_key, $needle_value, $cur_option, $elems_left)))
+           || (0 == $elems_left));
+
+  if (!($skip)) {
+    # We don't care about the values this time around.
+
+    my $option_key = q{};
+
+    foreach my $tmp_option_key (keys (%{$cur_option})) {
+      $option_key = $tmp_option_key;
+    }
+
+    if ($option_key eq $needle_key) {
+      $ret = 1;
+    }
+  }
+
+  return $ret;
+}
+
+# Helper for a map operation on the intermediate options array.
 #
-# If only a key is specified, removes any entry that matches this key,
-# regardless of its value.
+# Takes the option-to-modify's key and value, the current element and amount of
+# elements left in the array as arguments and returns the modified element or
+# the original one, if modification was not necessary.
+sub filter_option_modify {
+  my $skip = 0;
+  my $needle_key = shift;
+  my $needle_value = shift;
+  my $cur_option = shift;
+  my $elems_left = shift;
+
+  my @ret = ( $cur_option );
+
+  $skip = ((!(sanitize_input_filter ($needle_key, $needle_value, $cur_option, $elems_left)))
+           || (0 == $elems_left));
+
+  if (!($skip)) {
+    my $option_key = q{};
+
+    foreach my $tmp_option_key (keys (%{$cur_option})) {
+      $option_key = $tmp_option_key;
+    }
+
+    if ($option_key eq $needle_key) {
+      my $new_opt = { };
+
+      # Don't add empty options as an empty string key with undef value; even
+      # though that's technically legit we want to represent this situation by
+      # an empty hash.
+      if (!(($needle_key) || (defined ($needle_value)))) {
+        print {*STDERR} "Empty option addition/modification requested, this is deprecated. Adding empty hash.\n";
+      }
+      else {
+        $new_opt->{$needle_key} = $needle_value;
+      }
+
+      @ret = ( $new_opt );
+    }
+  }
+
+  return @ret;
+}
+
+# Removes from, adds to or modifies an entry in the intermediate options array.
+#
+# Expects an intermediate options reference as its first parameter, a boolean
+# value which should be set to true for removals or false for
+# modifications/additions and the option-to-be-modified as a third parameter.
 #
-# If both a key and a value are specified, only matching combinations will be
-# removed from the array. That is, if the array already contains such a key
-# with either no value or a different value, it will be unaffected.
+# For removals, the function behaves like this:
+#   - If only a key is specified, removes any entry that matches this key,
+#     regardless of its value.
+#   - If both a key and a value are specified, only matching combinations will
+#     be removed from the array. That is, if the array already contains such a
+#     key with either no value or a different value, it will be unaffected.
 #
-# Returns a reference to a modified copy of the intermediate options array.
+# Additions or modifications are handled like this:
+#   - If a given key is part of the intermediate representation, all such
+#     occurrences will be replaced by the new value.
+#   - Otherwise, the new value will be added at the end of the intermediate
+#     representation.
+#
+# Returns a reference to a modified *copy* of the intermediate options array.
 #
 # On error, returns a reference to undef.
-sub remove_option {
+sub transform_intermediate {
   my $ret = undef;
   my $error_detected = 0;
 
   my $options = shift;
+  my $remove = shift;
   my $option = shift;
 
   if ('ARRAY' ne ref ($options)) {
@@ -406,20 +533,81 @@ sub remove_option {
   }
 
   if (!($error_detected)) {
+    if (!(defined ($remove))) {
+      print {*STDERR} "Invalid mode option boolean passed, erroring out.\n";
+      $error_detected = 1;
+    }
+  }
+
+  if (!($error_detected)) {
     if (!(defined ($option))) {
-      print {*STDERR} "No or invalid option to be removed passed, erroring out.\n";
+      print {*STDERR} "No or invalid new option passed, erroring out.\n";
       $error_detected = 1;
     }
   }
 
+  my $work_option_key = undef;
+  my $work_option_value = undef;
+
+  if (!($error_detected)) {
+    my $work_opt_kv = sanitize_workoption_filter ($option);
+
+    if (!(defined ($work_opt_kv))) {
+      print {*STDERR} "Unable to split up working option into key and value pair, returning undef.\n";
+      $error_detected = 1;
+    }
+    else {
+      $work_option_key = shift (@{$work_opt_kv});
+      $work_option_value = shift (@{$work_opt_kv});
+    }
+  }
+
   if (!($error_detected)) {
     # Set return value to a *deep copy* of our original array.
     $ret = dclone ($options);
 
     my $elements_left = @{$ret};
 
-    # Let the filter function handle the actual work.
-    @{$ret} = grep { filter_option_remove ($option, $_, --$elements_left) } (@{$ret});
+    if ($remove) {
+      # Let the filter function handle the actual work.
+      @{$ret} = grep { filter_option_remove ($work_option_key, $work_option_value, $_, --$elements_left) } (@{$ret});
+
+      # Check to see if the intermediate representation is empty now (save for
+      # the display port entry) and add an empty element if it is.
+      if (1 == scalar (@{$ret})) {
+        print {*STDERR} "Removal operation led to option string being empty, adding empty element though deprecated.\n";
+        unshift (@{$ret}, { });
+      }
+    }
+    else {
+      # Yes, grep () isn't a great choice for a boolean comparison. It will do
+      # what we need just fine, but doesn't short-circuit after finding the
+      # first match, hence uselessly continuing through the full array.
+      # List::MoreUtils::any would be more appropriate here, but this would add
+      # another dependency and option strings are pretty small, so don't
+      # overoptimize here.
+      if (scalar (grep { filter_find_key ($work_option_key, $work_option_value, $_, --$elements_left) } (@{$ret}))) {
+        # Such an option already exists, we'll modify all occurrences.
+        $elements_left = @{$ret};
+        $ret = [ map { filter_option_modify ($work_option_key, $work_option_value, $_, --$elements_left) } (@{$ret}) ];
+      }
+      else {
+        my $new_opt = { $work_option_key => $work_option_value };
+
+        # No such option exists, we'll add it to the end of the current
+        # options.
+        # At least in theory. Practically, there's one special case: if the
+        # only "real" element is an empty one, for instance because the option
+        # string was empty to begin with save the display port specifier, then
+        # we want to replace this option instead.
+        if ((2 == scalar (@{$ret})) && (!(scalar (keys (%{$ret->[0]}))))) {
+          splice (@{$ret}, 0, 1, $new_opt);
+        }
+        else {
+          splice (@{$ret}, -1, 0, $new_opt);
+        }
+      }
+    }
   }
 
   return $ret;
@@ -443,9 +631,19 @@ print {*STDERR} Dumper (intermediate_to_string ($intermediate)) . "\n";
 
 my $option_to_remove = shift;
 
-print {*STDERR} Dumper (remove_option ($intermediate, $option_to_remove)) . "\n";
+my $intermediate_removed = transform_intermediate ($intermediate, 1, $option_to_remove);
+
+print {*STDERR} Dumper ($intermediate_removed) . "\n";
+
+print {*STDERR} Dumper (intermediate_to_string ($intermediate_removed)) . "\n";
+
+my $option_to_add = shift;
+
+my $intermediate_added = transform_intermediate ($intermediate_removed, 0, $option_to_add);
+
+print {*STDERR} Dumper ($intermediate_added) . "\n";
 
-print {*STDERR} Dumper (intermediate_to_string (remove_option ($intermediate, $option_to_remove))) . "\n";
+print {*STDERR} Dumper (intermediate_to_string ($intermediate_added)) . "\n";
 
 #my $value = shift;
 #my $allow_negative = shift;

--
Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/x2goserver.git


More information about the x2go-commits mailing list