Defect #11662

Invalid query returned from Issues.visible scope after accessing User#projects_by_role with a role that is not present

Added by Joe Rocklin over 5 years ago. Updated about 5 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:Issues
Target version:2.1.0
Resolution:Fixed Affected version:2.0.3

Description

I was seeing an issue with some users getting a 500 error from their 'My Page' view. The stack trace showed a mysql query error, with the offending part being:

... OR projects.id IN () ...

After some digging I discovered the problem was entering from the Project.allowed_to_condition when looping of the user.project_by_role list the user didn't have any projects specified with the given role. See attached patch.

rake about output:
About your application's environment
Ruby version 1.9.3 (x86_64-linux)
RubyGems version 1.8.24
Rack version 1.4
Rails version 3.2.6
Active Record version 3.2.6
Action Pack version 3.2.6
Active Resource version 3.2.6
Action Mailer version 3.2.6
Active Support version 3.2.6
Middleware Rack::Cache, ActionDispatch::Static, Rack::Lock, #<ActiveSupport::Cache::Strategy::LocalCache::Middleware:0x00000002271f30>, Rack::Runtime, Rack::MethodOverride, ActionDispatch::RequestId, Rails::Rack::Logger, ActionDispatch::ShowExceptions, ActionDispatch::DebugExceptions, ActionDispatch::RemoteIp, ActionDispatch::Callbacks, ActiveRecord::ConnectionAdapters::ConnectionManagement, ActiveRecord::QueryCache, ActionDispatch::Cookies, ActionDispatch::Session::CookieStore, ActionDispatch::Flash, RedmineDmsf::NoParse, ActionDispatch::ParamsParser, ActionDispatch::Head, Rack::ConditionalGet, Rack::ETag, ActionDispatch::BestStandardsSupport, OpenIdAuthentication
Application root /opt/redmine
Environment production
Database adapter mysql2
Database schema version 20120422150750

fix_empty_project.patch Magnifier - exclude conditions for a role if the user has no projects with that role (598 Bytes) Joe Rocklin, 2012-08-20 19:53

Associated revisions

Revision 10241
Added by Jean-Philippe Lang about 5 years ago

Make sure we don't build an empty IN () statement (#11662).

Revision 10242
Added by Jean-Philippe Lang about 5 years ago

Do not build a projects_by_role Hash that gets updated when accessing a key that is not present (#11662).

History

#1 Updated by Etienne Massip over 5 years ago

Could you please provide a less technical description and steps to reproduce?

#2 Updated by Joe Rocklin over 5 years ago

I guess the less technical description would be: "some users get a 500 error when trying to view 'My Page'" and the following error is in the log:

ActionView::Template::Error (Mysql2::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 (issues.project_id in (1,2,5,6,7,8,9,10,11,12,14,154) AND ((start_date>' at line 1: SELECT `issues`.`id` AS t0_r0, `issues`.`tracker_id` AS t0_r1, `issues`.`project_id` AS t0_r2, `issues`.`subject` AS t0_r3, `issues`.`description` AS t0_r4, `issues`.`due_date` AS t0_r5, `issues`.`category_id` AS t0_r6, `issues`.`status_id` AS t0_r7, `issues`.`assigned_to_id` AS t0_r8, `issues`.`priority_id` AS t0_r9, `issues`.`fixed_version_id` AS t0_r10, `issues`.`author_id` AS t0_r11, `issues`.`lock_version` AS t0_r12, `issues`.`created_on` AS t0_r13, `issues`.`updated_on` AS t0_r14, `issues`.`start_date` AS t0_r15, `issues`.`done_ratio` AS t0_r16, `issues`.`estimated_hours` AS t0_r17, `issues`.`parent_id` AS t0_r18, `issues`.`root_id` AS t0_r19, `issues`.`lft` AS t0_r20, `issues`.`rgt` AS t0_r21, `issues`.`is_private` AS t0_r22, `issues`.`position` AS t0_r23, `issues`.`remaining_hours` AS t0_r24, `issues`.`story_points` AS t0_r25, `projects`.`id` AS t1_r0, `projects`.`name` AS t1_r1, `projects`.`description` AS t1_r2, `projects`.`homepage` AS t1_r3, `projects`.`is_public` AS t1_r4, `projects`.`parent_id` AS t1_r5, `projects`.`created_on` AS t1_r6, `projects`.`updated_on` AS t1_r7, `projects`.`identifier` AS t1_r8, `projects`.`status` AS t1_r9, `projects`.`lft` AS t1_r10, `projects`.`rgt` AS t1_r11, `projects`.`dmsf_description` AS t1_r12, `trackers`.`id` AS t2_r0, `trackers`.`name` AS t2_r1, `trackers`.`is_in_chlog` AS t2_r2, `trackers`.`position` AS t2_r3, `trackers`.`is_in_roadmap` AS t2_r4, `enumerations`.`id` AS t3_r0, `enumerations`.`name` AS t3_r1, `enumerations`.`position` AS t3_r2, `enumerations`.`is_default` AS t3_r3, `enumerations`.`type` AS t3_r4, `enumerations`.`active` AS t3_r5, `enumerations`.`project_id` AS t3_r6, `enumerations`.`parent_id` AS t3_r7, `users`.`id` AS t4_r0, `users`.`login` AS t4_r1, `users`.`hashed_password` AS t4_r2, `users`.`firstname` AS t4_r3, `users`.`lastname` AS t4_r4, `users`.`mail` AS t4_r5, `users`.`admin` AS t4_r6, `users`.`status` AS t4_r7, `users`.`last_login_on` AS t4_r8, `users`.`language` AS t4_r9, `users`.`auth_source_id` AS t4_r10, `users`.`created_on` AS t4_r11, `users`.`updated_on` AS t4_r12, `users`.`type` AS t4_r13, `users`.`identity_url` AS t4_r14, `users`.`mail_notification` AS t4_r15, `users`.`salt` AS t4_r16, `users`.`reminder_notification` AS t4_r17 FROM `issues` LEFT OUTER JOIN `projects` ON `projects`.`id` = `issues`.`project_id` LEFT OUTER JOIN `trackers` ON `trackers`.`id` = `issues`.`tracker_id` LEFT OUTER JOIN `enumerations` ON `enumerations`.`id` = `issues`.`priority_id` AND `enumerations`.`type` IN ('IssuePriority') LEFT OUTER JOIN `users` ON `users`.`id` = `issues`.`assigned_to_id` WHERE (((projects.status=1 AND projects.id IN (SELECT em.project_id FROM enabled_modules em WHERE em.name='issue_tracking')) AND ((projects.is_public = 1 AND ((issues.is_private = 0 OR issues.author_id = 97 OR issues.assigned_to_id IN (97,6,27,34,42,48,56,68,90,91,98)))) OR (projects.id IN (6,9,7,1,5,154,2,11,12,10,8) AND ((issues.is_private = 0 OR issues.author_id = 97 OR issues.assigned_to_id IN (97,6,27,34,42,48,56,68,90,91,98)))) OR (projects.id IN (14) AND ((issues.is_private = 0 OR issues.author_id = 97 OR issues.assigned_to_id IN (97,6,27,34,42,48,56,68,90,91,98)))) OR projects.id IN ()))) AND (issues.project_id in (1,2,5,6,7,8,9,10,11,12,14,154) AND ((start_date>='2012-08-19' and start_date<='2012-08-25') or (due_date>='2012-08-19' and due_date<='2012-08-25')))):
    1: <h3><%= l(:label_calendar) %></h3>
    2:
    3: <% calendar = Redmine::Helpers::Calendar.new(Date.today, current_language, :week)
    4:    calendar.events = Issue.visible.find :all,
    5:                      :conditions => ["#{Issue.table_name}.project_id in (#{@user.projects.collect{|m| m.id}.join(',')}) AND ((start_date>=? and start_date<=?) or (due_date>=? and due_date<=?))", calendar.startdt, calendar.enddt, calendar.startdt, calendar.enddt],
    6:                      :include => [:project, :tracker, :priority, :assigned_to] unless @user.projects.empty? %>
    7:
  app/views/my/blocks/_calendar.html.erb:4:in `_app_views_my_blocks__calendar_html_erb___2203117175707133972_51672440'
  app/views/my/page.html.erb:11:in `block in _app_views_my_page_html_erb___3004501534925273256_28909740'
  app/views/my/page.html.erb:8:in `each'
  app/views/my/page.html.erb:8:in `_app_views_my_page_html_erb___3004501534925273256_28909740'

It seems to be happening for users which were not listed as 'Manager' for any project in our configuration. If you were a manager of at least one project, then the error was not seen.

I did some more testing this morning and found that the problem also goes away (without the patch applied and without being a manager of any project) if I move a custom block (from a plugin which provides an alternate calendar view) to the bottom of the page so it is the last thing rendered. The entirety of the code for the block is:

<h3><%= l(:label_personal_calendar) %></h3>

<% calendar = Redmine::Helpers::Calendar.new(Date.today, current_language, :week)
  mgr_projects = User.current.projects_by_role[Role.where(:name => "Manager").first]
  mgr_project_ids = [0]
  mgr_projects.each do |proj|
    mgr_project_ids << proj.id
  end

  watched = Issue.on_active_project.watched_by(User.current.id)
  rest = Issue.where("start_date BETWEEN :start_date AND :end_date OR due_date BETWEEN :start_date AND :end_date",
                  :start_date => calendar.startdt, :end_date => calendar.enddt)
              .where("author_id = :id OR assigned_to_id IN (:id_list) OR project_id IN (:mgr_list)",
                     :id => User.current.id,
                     :id_list => [User.current.id] + User.current.group_ids,
                     :mgr_list => mgr_project_ids)

  calendar.events = watched + rest;
%>

<%= render :partial => 'common/calendar', :locals => {:calendar => calendar } %>

#3 Updated by Etienne Massip over 5 years ago

  • Resolution set to Cant reproduce

Looking at User#project_by_role code, I can't see a way it could return a map containing a key with no values associated.

Could you try with no plugin installed?

If that doesn't change anything, maybe you could try to change source:/tags/2.0.3/app/models/user.rb#L426 if membership.project with if membership.project.present??

#4 Updated by Jean-Philippe Lang about 5 years ago

This is due to the way @projects_by_role is initialized in User#projects_by_role. Accessing projects_by_role[] with a key that does not exist implicitely creates an empty array as its value, as demonstrated below:

irb(main):001:0> a = Hash.new {|h,k| h[k]=[]}
=> {}
irb(main):002:0> a[:foo]
=> []
irb(main):003:0> a
=> {:foo=>[]}

#5 Updated by Jean-Philippe Lang about 5 years ago

  • Subject changed from Invalid query returned from Issues.visible scope to Invalid query returned from Issues.visible scope after accessing User#projects_by_role with a role that is not present
  • Status changed from New to Closed
  • Assignee set to Jean-Philippe Lang
  • Target version set to 2.1.0
  • Resolution changed from Cant reproduce to Fixed

This should be fixed by the 2 related commits.

#6 Updated by Etienne Massip about 5 years ago

Jean-Philippe Lang wrote:

This is due to the way @projects_by_role is initialized in User#projects_by_role. Accessing projects_by_role[] with a key that does not exist implicitely creates an empty array as its value, as demonstrated below:

Gosh, I didn't know that, thanks for the lesson.

Weird idea though :p

Also available in: Atom PDF