Defect #8371

MySQL error when filtering a custom field using the REST api

Added by Bruno Bigras over 6 years ago. Updated about 6 years ago.

Status:ClosedStart date:2011-05-13
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:REST API
Target version:1.3.0
Resolution:Fixed Affected version:

Description

MySQL 5.1.41
Redmine 1.1.3
ruby 1.8.7
rails 2.3.5

I created a custom field named 'sha1' (text format), 0 to 40 length.

I tested it with a dumb value 'aaa' and it worked fine with http://myhost/redmine/issues.xml?project_id=5&cf_1=aaa

I had an error when I tried to use it with a real sha1 hash.

I was able to reproduce the problem when trying to filter with a value of 'c' but it worked with 'a' and 'b'.
http://myhost/redmine/issues.xml?project_id=5&cf_1=a <-- works
http://myhost/redmine/issues.xml?project_id=5&cf_1=b <-- works
http://myhost/redmine/issues.xml?project_id=5&cf_1=c <-- doesn't works

I get from the log:

Processing IssuesController#index to xml (for 10.1.10.240 at 2011-05-13 13:43:19) [GET]
  Parameters: {"format"=>"xml", "project_id"=>"5", "action"=>"index", "controller"=>"issues", "cf_1"=>"c"}
Query::StatementInvalid: Mysql::Error: You have an error in your SQL syntax; 
check the manual that corresponds to your MySQL server version for the right
syntax to use near ') AND projects.id = 5 AND projects.status=1 AND 
projects.id IN (SELECT em.projec' at line 1:

There's no condition after the where at line 14. (note that I indented the query myself).

SELECT count(DISTINCT `issues`.id) AS count_all
FROM `issues`
LEFT OUTER JOIN `issue_statuses` ON `issue_statuses`.id = `issues`.status_id
LEFT OUTER JOIN `projects` ON `projects`.id = `issues`.project_id
WHERE
    ((issue_statuses.is_closed=0) AND
    issues.id IN (
        SELECT issues.id
        FROM issues
        LEFT OUTER JOIN custom_values ON
            custom_values.customized_type='Issue' AND
            custom_values.customized_id=issues.id AND
            custom_values.custom_field_id=1
        WHERE 
    ) AND
    projects.id = 5 AND
    projects.status=1 AND
    projects.id IN (SELECT em.project_id FROM enabled_modules em WHERE em.name='issue_tracking'))

2013_fix_custom_field_filter.patch Magnifier (770 Bytes) Bruno Bigras, 2011-05-19 16:37

short_filters_regexps.patch Magnifier (4.21 KB) Etienne Massip, 2011-05-20 15:23

filter_out_illegal_query_filter_values.patch Magnifier (1.98 KB) Etienne Massip, 2011-10-19 20:20

Associated revisions

Revision 7624
Added by Etienne Massip about 6 years ago

Fixed shot filter expression parsing depending upon field operators (#8371).

Revision 7625
Added by Etienne Massip about 6 years ago

Removed test part failing with PostgreSQL (#8371).

Revision 7627
Added by Etienne Massip about 6 years ago

Restored valid test part removed with r7625 (#8371).

Revision 7628
Added by Etienne Massip about 6 years ago

Filter out illegal values to prevent raise of PostgreSQL exceptions, restored last test part removed with r7625 (#8371).

History

#1 Updated by Bruno Bigras over 6 years ago

Also note that the sha1 I was trying to use with the filter also began with the letter 'c'.

#2 Updated by Bruno Bigras over 6 years ago

I figured out that the regex at query.rb#L270 (line 3 here) removes the 'c' (and other letters) from the filter value.

def add_short_filter(field, expression)
  return unless expression
  parms = expression.scan(/^(o|c|!\*|!|\*)?(.*)$/).first
  add_filter field, (parms[0] || "="), [parms[1] || ""]
end

http://www.redmine.org/projects/redmine/repository/entry/trunk/app/models/query.rb#L270

Here's a diff that fixes my problem. I guess a better solution would be to only use the regex when it's needed.

Index: redmine-1.1.3/app/models/query.rb
===================================================================
--- redmine-1.1.3.orig/app/models/query.rb    2011-04-29 05:33:42.000000000 -0400
+++ redmine-1.1.3/app/models/query.rb    2011-05-19 10:00:21.265646521 -0400
@@ -254,8 +254,13 @@

   def add_short_filter(field, expression)
     return unless expression
-    parms = expression.scan(/^(o|c|!\*|!|\*)?(.*)$/).first
-    add_filter field, (parms[0] || "="), [parms[1] || ""]
+    if field =~ /^cf_(\d+)$/
+      # custom field
+      add_filter field, "=", [expression]
+    else
+      parms = expression.scan(/^(o|c|!\*|!|\*)?(.*)$/).first
+      add_filter field, (parms[0] || "="), [parms[1] || ""]
+    end
   end

#3 Updated by Etienne Massip over 6 years ago

  • Target version set to Candidate for next minor release

Nice catch !

#4 Updated by Etienne Massip over 6 years ago

  • File short_filters_regexps.patch added

Tried to write a more generic patch.

#5 Updated by Etienne Massip over 6 years ago

Previous patch was incomplete.

#6 Updated by Etienne Massip over 6 years ago

  • File deleted (short_filters_regexps.patch)

#7 Updated by Bruno Bigras over 6 years ago

Etienne Massip wrote:

Previous patch was incomplete.

The patch seems to work perfectly. I'll report back if I have any problem later.

Thanks!

#8 Updated by Etienne Massip about 6 years ago

  • Target version changed from Candidate for next minor release to 1.3.0

BTW I'm not sure it's worth it to maintain such complexity in request arguments processing as there is another way to do so (via f[], o[], and v[] arguments), maybe should we stick to handling equality.

I'm Ok with reverting these changes if needed.

#9 Updated by Etienne Massip about 6 years ago

  • Status changed from New to Resolved
  • Resolution set to Fixed

#10 Updated by Etienne Massip about 6 years ago

I reverted r7628 because of failing tests in QueryTest unit test.

The tests failed because the values used in applied filters are not values that are displayed in the issue query form.

I attach the patch for history.

Anyway, the commit was not immediatly related to this issue, which remains Fixed.

#11 Updated by Jean-Philippe Lang about 6 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF