[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