[Rt-commit] rt branch, 4.2/multi-value-cf-in-rest, created. rt-4.2.9-97-g82a0d98

Alex Vandiver alexmv at bestpractical.com
Mon Feb 2 21:55:57 EST 2015


The branch, 4.2/multi-value-cf-in-rest has been created
        at  82a0d98af204af4062d0d7118035f09a0d27c6a7 (commit)

- Log -----------------------------------------------------------------
commit eebb44a15a49a47d89115e2fbebc80a65d752af3
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Jul 6 15:39:30 2011 -0400

    Add (failing) tests for escaping

diff --git a/t/web/command_line.t b/t/web/command_line.t
index c02c5e9..227ac78 100644
--- a/t/web/command_line.t
+++ b/t/web/command_line.t
@@ -265,6 +265,55 @@ expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
 expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
 expect_like(qr/\QCF.{MultipleCF$$}\E: '1,2\\'s,3'/i, 'Verified change');
 
+# Test escaping of quotes - generate (foo)(bar') with no escapes
+my $ticket = RT::Ticket->new($RT::SystemUser);
+expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo',bar'\"", "Changing CF with no slashes: 'foo',bar'");
+expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+$ticket->Load($ticket_id);
+is($ticket->CustomFieldValuesAsString("MultipleCF$$"), "foo"."\n"."bar'", "Direct value checks out");
+expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=\"bar'\"", "Stripping off bar' (should be present)");
+expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
+expect_like(qr/CF\.{MultipleCF$$}: foo/i, 'Verified change');
+
+# With one \, generate (foo',bar)
+expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo\\',bar'\"", "Changing CF to one slash: 'foo\\',bar'");
+expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+$ticket->Load($ticket_id);
+is($ticket->CustomFieldValuesAsString("MultipleCF$$"), "foo',bar", "Direct value checks out");
+expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=\"bar'\"", "Stripping off bar' (should _not_ be present)");
+expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
+expect_like(qr/CF\.{MultipleCF$$}: 'foo\\',bar'/i, 'Verified change');
+
+# With two \, generate (foo\)(bar')
+expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo\\\\',bar'\"", "Changing CF to two slashes: 'foo\\\\',bar'");
+expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+$ticket->Load($ticket_id);
+is($ticket->CustomFieldValuesAsString("MultipleCF$$"), "foo\\"."\n"."bar'", "Direct value checks out");
+expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=\"bar'\"", "Stripping off bar' (should be present)");
+expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
+expect_like(qr/CF\.{MultipleCF$$}: 'foo\\\\'/i, 'Verified change');
+
+# With three \, generate (foo\',bar)
+expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo\\\\\\',bar'\"", "Changing CF to three slashes: 'foo\\\\\\',bar'");
+expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+$ticket->Load($ticket_id);
+is($ticket->CustomFieldValuesAsString("MultipleCF$$"), "foo\\'bar", "Direct value checks out");
+expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=\"bar'\"", "Stripping off bar' (should _not_ be present)");
+expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
+expect_like(qr/CF\.{MultipleCF$$}: 'foo\\\\\\',bar'/i, 'Verified change');
+
+# Check that we don't infinite-loop on 'foo'bar,baz
+TODO: {
+    todo_skip "Quotes not at comma boundries infinite-loop", 2;
+    expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo'bar,baz\"", 'Changing CF to have quotes not at commas');
+    expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+}
+
+
 # ...
 # change a ticket's ...[other properties]...
 # ...

commit 4a7e19cbac235b601951cb0cab44beea6e2b035e
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 11 12:40:35 2011 -0400

    Update the tests to be more thorough

diff --git a/t/web/command_line.t b/t/web/command_line.t
index 227ac78..c4e7ec0 100644
--- a/t/web/command_line.t
+++ b/t/web/command_line.t
@@ -213,7 +213,7 @@ expect_send("edit ticket/$ticket_id set CF-myCF$$=1,2,3", 'Changing CF...');
 expect_like(qr/Ticket $ticket_id updated/, 'Changed cf');
 expect_send("show ticket/$ticket_id -f CF-myCF$$", 'Checking new value');
 expect_like(qr/\QCF.{myCF$$}\E: 1,2,3/i, 'Verified change');
-expect_send("edit ticket/$ticket_id set CF-myCF$$=\"1's,2,3\"", 'Changing CF...');
+expect_send(qq{edit ticket/$ticket_id set CF-myCF$$="1's,2,3"}, 'Changing CF...');
 expect_like(qr/Ticket $ticket_id updated/, 'Changed cf');
 expect_send("show ticket/$ticket_id -f CF-myCF$$", 'Checking new value');
 expect_like(qr/\QCF.{myCF$$}\E: 1's,2,3/i, 'Verified change');
@@ -238,79 +238,86 @@ expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
 expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
 expect_like(qr/\QCF.{MultipleCF$$}\E: b,\s*c,\s*o/i, 'Verified multiple cf change');
 
-expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'a,b,c'\" ", 'Changing CF...');
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
-expect_like(qr/\QCF.{MultipleCF$$}\E: 'a,b,c'/i, 'Verified change');
-expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=a", 'Changing CF...');
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
-expect_like(qr/\QCF.{MultipleCF$$}\E: 'a,b,c'/i, 'Verified change');
+sub multi_round_trip {
+    my ($op, $value, $regex) = @_;
+    $Test::Builder::Level++;
+    # The outer double quotes are for the argument parsing that the
+    # command-line does; the extra layer of escaping is to for them, as
+    # well.  It is equivilent to the quoting that the shell would
+    # require.
+    my $quoted = $value;
+    $quoted =~ s/(["\\])/\\$1/g;
+    expect_send(qq{edit ticket/$ticket_id $op CF.{MultipleCF$$}="$quoted"}, qq{CF $op $value});
+    expect_like(qr/Ticket $ticket_id updated/,                              qq{Got expected "updated" answer});
+    expect_send(qq{show ticket/$ticket_id -f CF.{MultipleCF$$}},            qq{Sent "show"});
+    expect_like(qr/\QCF.{MultipleCF$$}\E: $regex$/i,                        qq{Answer matches $regex});
+}
 
-expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=q{a,b,c}", 'Changing CF...');
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
-expect_like(qr/\QCF.{MultipleCF$$}\E: 'a,b,c'/i, 'Verified change');
-expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=a", 'Changing CF...');
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
-expect_like(qr/\QCF.{MultipleCF$$}\E: 'a,b,c'/i, 'Verified change');
-expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=\"'a,b,c'\"", 'Changing CF...');
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
-expect_like(qr/\QCF.{MultipleCF$$}\E: \s*$/i, 'Verified change');
+# Test simple quoting
+my $ticket = RT::Ticket->new($RT::SystemUser);
+$ticket->Load($ticket_id);
+multi_round_trip(set => q|'a,b,c'|,  qr/'a,b,c'/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 1,     "Has only one CF value");
+is($ticket->FirstCustomFieldValue("MultipleCF$$"), q{a,b,c}, "And that CF value is as expected");
 
-expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"q{1,2's,3}\"", 'Changing CF...');
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
-expect_like(qr/\QCF.{MultipleCF$$}\E: '1,2\\'s,3'/i, 'Verified change');
+multi_round_trip(del => q|a|,        qr/'a,b,c'/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 1,     "Still has only one CF value");
+is($ticket->FirstCustomFieldValue("MultipleCF$$"), q{a,b,c}, "And that CF value is as expected");
+
+multi_round_trip(set => q|q{a,b,c}|, qr/'a,b,c'/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 1, "Still has only one CF value");
+is($ticket->FirstCustomFieldValue("MultipleCF$$"), q{a,b,c}, "And that CF value is as expected");
+
+multi_round_trip(del => q|a|,        qr/'a,b,c'/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 1, "Still has only one CF value");
+is($ticket->FirstCustomFieldValue("MultipleCF$$"), q{a,b,c}, "And that CF value is as expected");
+
+multi_round_trip(del => q|'a,b,c'|,  qr/\s*/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 0, "Now has no CF values");
+
+multi_round_trip(set => q|q{1,2's,3}|, qr/'1,2\\'s,3'/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 1, "Still has only one CF value");
+is($ticket->FirstCustomFieldValue("MultipleCF$$"), q{1,2's,3}, "And that CF value is as expected");
+
+multi_round_trip(del => q|q{1,2's,3}|, qr/\s*/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 0, "Now has no CF values");
 
 # Test escaping of quotes - generate (foo)(bar') with no escapes
-my $ticket = RT::Ticket->new($RT::SystemUser);
-expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo',bar'\"", "Changing CF with no slashes: 'foo',bar'");
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-$ticket->Load($ticket_id);
-is($ticket->CustomFieldValuesAsString("MultipleCF$$"), "foo"."\n"."bar'", "Direct value checks out");
-expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=\"bar'\"", "Stripping off bar' (should be present)");
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
-expect_like(qr/CF\.{MultipleCF$$}: foo/i, 'Verified change');
+multi_round_trip(set => q|'foo',bar'|, qr/foo,bar'/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 2, "Has two values");
+is($ticket->CustomFieldValues("MultipleCF$$")->First->Content, q|foo|,  "Direct value checks out");
+is($ticket->CustomFieldValues("MultipleCF$$")->Last->Content,  q|bar'|, "Direct value checks out");
+multi_round_trip(del => q|bar'|,       qr/foo/);
 
 # With one \, generate (foo',bar)
-expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo\\',bar'\"", "Changing CF to one slash: 'foo\\',bar'");
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-$ticket->Load($ticket_id);
-is($ticket->CustomFieldValuesAsString("MultipleCF$$"), "foo',bar", "Direct value checks out");
-expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=\"bar'\"", "Stripping off bar' (should _not_ be present)");
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
-expect_like(qr/CF\.{MultipleCF$$}: 'foo\\',bar'/i, 'Verified change');
+
+# We obviously need two \s in the following q|| string in order to
+# generate a string with one actual \ in it; this causes the string to,
+# in general, have twice as many \s in it as we wish to test.
+multi_round_trip(set => q|'foo\\',bar'|, qr/'foo\\',bar'/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 1, "Has one value");
+is($ticket->CustomFieldValues("MultipleCF$$")->First->Content, q|foo',bar|,  "Direct value checks out");
 
 # With two \, generate (foo\)(bar')
-expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo\\\\',bar'\"", "Changing CF to two slashes: 'foo\\\\',bar'");
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-$ticket->Load($ticket_id);
-is($ticket->CustomFieldValuesAsString("MultipleCF$$"), "foo\\"."\n"."bar'", "Direct value checks out");
-expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=\"bar'\"", "Stripping off bar' (should be present)");
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
-expect_like(qr/CF\.{MultipleCF$$}: 'foo\\\\'/i, 'Verified change');
+multi_round_trip(set => q|'foo\\\\',bar'|, qr/foo\\,bar'/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 2, "Has two values");
+is($ticket->CustomFieldValues("MultipleCF$$")->First->Content, q|foo\\|,  "Direct value checks out");
+is($ticket->CustomFieldValues("MultipleCF$$")->Last->Content,  q|bar'|, "Direct value checks out");
+multi_round_trip(del => q|bar'|,           qr/foo\\/);
 
 # With three \, generate (foo\',bar)
-expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo\\\\\\',bar'\"", "Changing CF to three slashes: 'foo\\\\\\',bar'");
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-$ticket->Load($ticket_id);
-is($ticket->CustomFieldValuesAsString("MultipleCF$$"), "foo\\'bar", "Direct value checks out");
-expect_send("edit ticket/$ticket_id del CF.{MultipleCF$$}=\"bar'\"", "Stripping off bar' (should _not_ be present)");
-expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-expect_send("show ticket/$ticket_id -f CF.{MultipleCF$$}", 'Checking new value');
-expect_like(qr/CF\.{MultipleCF$$}: 'foo\\\\\\',bar'/i, 'Verified change');
+multi_round_trip(set => q|'foo\\\\\\',bar'|, qr/'foo\\\\\\',bar'/);
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 1, "Has one value");
+is($ticket->CustomFieldValues("MultipleCF$$")->First->Content, q|foo\\',bar|,  "Direct value checks out");
 
-# Check that we don't infinite-loop on 'foo'bar,baz
+# Check that we don't infinite-loop on 'foo'bar,baz; this should be ('foo'bar)(baz)
 TODO: {
-    todo_skip "Quotes not at comma boundries infinite-loop", 2;
+    todo_skip "Quotes not at comma boundries infinite-loop", 5;
     expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo'bar,baz\"", 'Changing CF to have quotes not at commas');
     expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+    is($ticket->CustomFieldValues("MultipleCF$$")->Count, 2, "Has two value");
+    is($ticket->CustomFieldValues("MultipleCF$$")->First->Content, q|'foo'bar|,  "Direct value checks out");
+    is($ticket->CustomFieldValues("MultipleCF$$")->Last->Content, q|baz|,  "Direct value checks out");
 }
 
 

commit 8da086ca2e69ecb22e828d4cd73de58f6329f6c3
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 11 12:41:39 2011 -0400

    Compare base64 versions of the data, soas to not spew binary data on failure

diff --git a/t/web/command_line.t b/t/web/command_line.t
index c4e7ec0..1a3b43d 100644
--- a/t/web/command_line.t
+++ b/t/web/command_line.t
@@ -592,7 +592,11 @@ sub check_attachment {
     TODO: {
         local $TODO = "Binary PNG content is getting mangled somewhere along the way"
             if $attachment_path =~ /\.png$/;
-        expect_is($attachment_content,"Attachment contains original text");
+        is(
+            MIME::Base64::encode_base64(Test::Expect::before()),
+            MIME::Base64::encode_base64($attachment_content),
+            "Attachment contains original text"
+       );
     }
 }
 

commit a4ce86f1dca23057c7ddea8446062ca98133ff94
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 11 12:15:17 2011 -0400

    Only escape the output if we're going to quote it

diff --git a/share/html/REST/1.0/Forms/ticket/default b/share/html/REST/1.0/Forms/ticket/default
index dea2a8b..f7cfac2 100644
--- a/share/html/REST/1.0/Forms/ticket/default
+++ b/share/html/REST/1.0/Forms/ticket/default
@@ -282,8 +282,8 @@ if (!keys(%data)) {
         else {
             while (my $v = $vals->Next()) {
                 my $content = $v->Content;
-                $content =~ s/'/\\'/g;
                 if ( $v->Content =~ /,/ ) {
+                    $content =~ s/([\\'])/\\$1/g;
                     push @out, q{'} . $content . q{'};
                 }
                 else {

commit 577ecd5f3a8fc0a7c5a4521d0e423c4664dabae8
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 11 12:15:46 2011 -0400

    Note that the vsplit code is duplicated

diff --git a/bin/rt.in b/bin/rt.in
index 8445738..d30326e 100644
--- a/bin/rt.in
+++ b/bin/rt.in
@@ -1591,6 +1591,8 @@ sub vpush {
     }
 }
 
+# WARNING: this code is duplicated in lib/RT/Interface/REST.pm
+# If you change one, change both functions at once
 # "Normalise" a hash key that's known to be multi-valued.
 sub vsplit {
     my ($val) = @_;

commit 3355b9458152d5555dcab5eea85d56f7e51f32d9
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 11 12:16:51 2011 -0400

    Move to a regex-based quoting parser for vsplit
    
    This also merges the 2.5 versions of the code.

diff --git a/bin/rt.in b/bin/rt.in
index d30326e..8f9d0cd 100644
--- a/bin/rt.in
+++ b/bin/rt.in
@@ -1595,47 +1595,52 @@ sub vpush {
 # If you change one, change both functions at once
 # "Normalise" a hash key that's known to be multi-valued.
 sub vsplit {
-    my ($val) = @_;
-    my ($word, @words);
-    my @values = ref $val eq 'ARRAY' ? @$val : $val;
-
-    foreach my $line (map {split /\n/} @values) {
-        # XXX: This should become a real parser, à la Text::ParseWords.
-        $line =~ s/^\s+//;
-        $line =~ s/\s+$//;
-        my ( $a, $b ) = split /\s*,\s*/, $line, 2;
-
-        while ($a) {
-            no warnings 'uninitialized';
-            if ( $a =~ /^'/ ) {
-                my $s = $a;
-                while ( $a !~ /'$/ || (   $a !~ /(\\\\)+'$/
-                            && $a =~ /(\\)+'$/ )) {
-                    ( $a, $b ) = split /\s*,\s*/, $b, 2;
-                    $s .= ',' . $a;
-                }
-                push @words, $s;
-            }
-            elsif ( $a =~ /^q\{/ ) {
-                my $s = $a;
-                while ( $a !~ /\}$/ ) {
-                    ( $a, $b ) =
-                      split /\s*,\s*/, $b, 2;
-                    $s .= ',' . $a;
-                }
-                $s =~ s/^q\{/'/;
-                $s =~ s/\}/'/;
-                push @words, $s;
+    my ($val, $strip) = @_;
+    my @words;
+    my @values = map {split /\n/} (ref $val eq 'ARRAY' ? @$val : $val);
+
+    foreach my $line (@values) {
+        while ($line =~ /\S/) {
+            $line =~ s/^
+                       \s*   # Trim leading whitespace
+                       (?:
+                           (")   # Quoted string
+                           ((?>[^\\"]*(?:\\.[^\\"]*)*))"
+                       |
+                           (')   # Single-quoted string
+                           ((?>[^\\']*(?:\\.[^\\']*)*))'
+                       |
+                           q\{(.*?)\} # A perl-ish q{} string; this does
+                                      # no paren balancing, however, and
+                                      # only exists for back-compat
+                       |
+                           (.*?)     # Anything else, until the next comma
+                       )
+                       \s*   # Trim trailing whitespace
+                       (?:
+                           \Z  # Finish at end-of-line
+                       |
+                           ,   # Or a comma
+                       )
+                      //xs or last; # There should be no way this match
+                                    # fails, but add a failsafe to
+                                    # prevent infinite-looping if it
+                                    # somehow does.
+            my ($quote, $quoted) = ($1 ? ($1, $2) : $3 ? ($3, $4) : ('', $5 || $6));
+            # Only unquote the quote character, or the backslash -- and
+            # only if we were originally quoted..
+            if ($5) {
+                $quoted =~ s/([\\'])/\\$1/g;
+                $quote = "'";
             }
-            else {
-                push @words, $a;
+            if ($strip) {
+                $quoted =~ s/\\([\\$quote])/$1/g if $quote;
+                push @words, $quoted;
+            } else {
+                push @words, "$quote$quoted$quote";
             }
-            ( $a, $b ) = split /\s*,\s*/, $b, 2;
         }
-
-
     }
-
     return \@words;
 }
 
diff --git a/lib/RT/Interface/REST.pm b/lib/RT/Interface/REST.pm
index edfc5d3..7e7027a 100644
--- a/lib/RT/Interface/REST.pm
+++ b/lib/RT/Interface/REST.pm
@@ -283,17 +283,52 @@ sub vpush {
 
 # "Normalise" a hash key that's known to be multi-valued.
 sub vsplit {
-    my ($val) = @_;
+    my ($val, $strip) = @_;
     my @words;
-
-    foreach my $line (map {split /\n/} (ref $val eq 'ARRAY') ? @$val : ($val||''))
-    {
-        # XXX: This should become a real parser, ? la Text::ParseWords.
-        $line =~ s/^\s+//;
-        $line =~ s/\s+$//;
-        push @words, split /\s*,\s*/, $line;
+    my @values = map {split /\n/} (ref $val eq 'ARRAY' ? @$val : $val);
+
+    foreach my $line (@values) {
+        while ($line =~ /\S/) {
+            $line =~ s/^
+                       \s*   # Trim leading whitespace
+                       (?:
+                           (")   # Quoted string
+                           ((?>[^\\"]*(?:\\.[^\\"]*)*))"
+                       |
+                           (')   # Single-quoted string
+                           ((?>[^\\']*(?:\\.[^\\']*)*))'
+                       |
+                           q{(.*?)}  # A perl-ish q{} string; this does
+                                     # no paren balancing, however, and
+                                     # only exists for back-compat
+                       |
+                           (.*?)     # Anything else, until the next comma
+                       )
+                       \s*   # Trim trailing whitespace
+                       (?:
+                           \Z  # Finish at end-of-line
+                       |
+                           ,   # Or a comma
+                       )
+                      //xs or last; # There should be no way this match
+                                    # fails, but add a failsafe to
+                                    # prevent infinite-looping if it
+                                    # somehow does.
+            my ($quote, $quoted) = ($1 ? ($1, $2) : $3 ? ($3, $4) : ('', $5 || $6));
+            # Only unquote the quote character, or the backslash -- and
+            # only if we were originally quoted..
+            if ($5) {
+                $quoted =~ s/([\\'])/\\$1/g;
+                $quote = "'";
+            }
+            if ($strip) {
+                $quoted =~ s/\\([\\$quote])/$1/g if $quote;
+                push @words, $quoted;
+            } else {
+                push @words, "$quote$quoted$quote";
+            }
+        }
     }
-
     return \@words;
 }
 
diff --git a/share/html/REST/1.0/Forms/ticket/default b/share/html/REST/1.0/Forms/ticket/default
index f7cfac2..27989cd 100644
--- a/share/html/REST/1.0/Forms/ticket/default
+++ b/share/html/REST/1.0/Forms/ticket/default
@@ -415,37 +415,7 @@ else {
                     $s =~ s/^# // if defined $s;
                 }
                 else {
-                    my @new;
-                    my ( $a, $b ) = split /\s*,\s*/, $val, 2;
-                    while ($a) {
-                        no warnings 'uninitialized';
-                        if ( $a =~ /^'/ ) {
-                            my $s = $a;
-                            while ( $a !~ /'$/ || ( $a !~ /(\\\\)+'$/
-                                            && $a =~ /(\\)+'$/ ) ) {
-                                ( $a, $b ) = split /\s*,\s*/, $b, 2;
-                                $s .= ',' . $a;
-                            }
-                            $s =~ s/^'//;
-                            $s =~ s/'$//;
-                            $s =~ s/\\'/'/g;
-                            push @new, $s;
-                        }
-                        elsif ( $a =~ /^q\{/ ) {
-                            my $s = $a;
-                            while ( $a !~ /\}$/ ) {
-                                ( $a, $b ) = split /\s*,\s*/, $b, 2;
-                                $s .= ',' . $a;
-                            }
-                            $s =~ s/^q\{//;
-                            $s =~ s/\}//;
-                            push @new, $s;
-                        }
-                        else {
-                            push @new, $a;
-                        }
-                        ( $a, $b ) = split /\s*,\s*/, $b, 2;
-                    }
+                    my @new = @{vsplit($val, 1)};
 
                     my %new;
                     $new{$_}++ for @new;

commit 82a0d98af204af4062d0d7118035f09a0d27c6a7
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 11 12:40:58 2011 -0400

    Un-TODO the tests

diff --git a/t/web/command_line.t b/t/web/command_line.t
index 1a3b43d..47f6728 100644
--- a/t/web/command_line.t
+++ b/t/web/command_line.t
@@ -311,14 +311,11 @@ is($ticket->CustomFieldValues("MultipleCF$$")->Count, 1, "Has one value");
 is($ticket->CustomFieldValues("MultipleCF$$")->First->Content, q|foo\\',bar|,  "Direct value checks out");
 
 # Check that we don't infinite-loop on 'foo'bar,baz; this should be ('foo'bar)(baz)
-TODO: {
-    todo_skip "Quotes not at comma boundries infinite-loop", 5;
-    expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo'bar,baz\"", 'Changing CF to have quotes not at commas');
-    expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
-    is($ticket->CustomFieldValues("MultipleCF$$")->Count, 2, "Has two value");
-    is($ticket->CustomFieldValues("MultipleCF$$")->First->Content, q|'foo'bar|,  "Direct value checks out");
-    is($ticket->CustomFieldValues("MultipleCF$$")->Last->Content, q|baz|,  "Direct value checks out");
-}
+expect_send("edit ticket/$ticket_id set CF.{MultipleCF$$}=\"'foo'bar,baz\"", 'Changing CF to have quotes not at commas');
+expect_like(qr/Ticket $ticket_id updated/, 'Changed multiple cf');
+is($ticket->CustomFieldValues("MultipleCF$$")->Count, 2, "Has two value");
+is($ticket->CustomFieldValues("MultipleCF$$")->First->Content, q|'foo'bar|,  "Direct value checks out");
+is($ticket->CustomFieldValues("MultipleCF$$")->Last->Content, q|baz|,  "Direct value checks out");
 
 
 # ...

-----------------------------------------------------------------------


More information about the rt-commit mailing list