[Rt-commit] rt branch, 4.4/tickets-invalid-query, created. rt-4.4.2-154-g994edf737

[? sunnavy]

The branch, 4.4/tickets-invalid-query has been created
        at  994edf737c8d2fe3ab0826929d6bc5344a8b079e (commit)

- Log -----------------------------------------------------------------
commit b6af27d94d867ef6e3fcd2b47d32451c29e1a14a
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu Mar 1 21:37:32 2018 +0800

    Fix the issue that invalid queries to FromSQL could pass validation
    
    An invalid query like:
    
        Status = 'open' and LastUpdated < yesterday
    
    could pass validation and be wrongly parsed as:
    
        SELECT main.* FROM Tickets main WHERE (main.IsMerged IS NULL) AND (main.Type = 'ticket') AND (main.Status = 'open')
    
    Since b68c84f0(Switch to parsing into a parse tree as an IR),
    RT::Tickets::_parser uses RT::Interface::Web::QueryBuilder::Tree to
    validate and parse queries, which doesn't "die" but returns errors if
    there are any found.
    
    _parser should directly "die" if errors are found in ::Tree->ParseSQL,
    especially that RT::Tickets::FromSQL relies on this "die" behavior to
    determine if the query is valid or not.

diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 73bf7f563..5d1570a80 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -3029,10 +3029,11 @@ sub _parser {
 
     require RT::Interface::Web::QueryBuilder::Tree;
     my $tree = RT::Interface::Web::QueryBuilder::Tree->new;
-    $tree->ParseSQL(
+    my @results = $tree->ParseSQL(
         Query => $string,
         CurrentUser => $self->CurrentUser,
     );
+    die join "; ", map { ref $_ eq 'ARRAY' ? $_->[ 0 ] : $_ } @results if @results;
 
     state ( $active_status_node, $inactive_status_node );
 

commit 994edf737c8d2fe3ab0826929d6bc5344a8b079e
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue Apr 17 16:44:18 2018 +0800

    Test invalid query for RT::Tickets::FromSQL

diff --git a/t/api/tickets.t b/t/api/tickets.t
index 172965c3b..407981baa 100644
--- a/t/api/tickets.t
+++ b/t/api/tickets.t
@@ -3,6 +3,7 @@ use strict;
 use warnings;
 use RT;
 use RT::Test tests => undef;
+use Test::Warn;
 
 {
 
@@ -144,4 +145,20 @@ ok( $unlimittickets->Count > 0, "UnLimited tickets object should return tickets"
     undef $count;
 }
 
+{
+    my $tickets = RT::Tickets->new( RT->SystemUser );
+    my ( $ret, $msg );
+    warning_like {
+        ( $ret, $msg ) = $tickets->FromSQL( "LastUpdated < yesterday" );
+    }
+    qr/Couldn't parse query: Wrong query, expecting a VALUE in 'LastUpdated < >yesterday<--here'/;
+
+    ok( !$ret, 'Invalid query' );
+    like(
+        $msg,
+        qr/Wrong query, expecting a VALUE in 'LastUpdated < >yesterday<--here'/,
+        'Invalid query message'
+    );
+}
+
 done_testing;

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