Patch #25570

unchecked use of params[:query]

Added by Jens Krämer about 5 years ago. Updated over 4 years ago.

Status:NewStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Code cleanup/refactoring
Target version:-

Description

Requests to URLs like /issues?query=12 result in an application error because query.rb expects params[:query] to be a hash, which it isn't in this case.

I know that there are a more instances in the code base where we assume params[:foo] to be a hash without explicit type checking. This patch does not attempt to address this problem on a global level. For whatever reason it happens to us quite often in this particular place, causing false alerts in server monitoring.

This patch adds an explicit type check to resolve this params[:query] case. As a bonus it makes the following 4 lines where values are taken out of the hash a bit nicer.

In general I think it would be preferably to raise an error of the 4xx class in such cases. Are there any plans to make use of Rails' Strong Parameters feature in the future? I think using these permit / require calls on parameters in controllers would catch such wrong types early and would lead to responses with a more appropriate error code automatically.

0001-adds-params-query-type-check.patch Magnifier (1.3 KB) Jens Krämer, 2017-04-11 06:53

History

#1 Updated by Jens Krämer about 5 years ago

wherever I wrote 'to be a hash', I mean 'accessible like a hash'. It's an ActionController::Paramaters instance that we want.

#2 Updated by Jens Krämer over 4 years ago

As it turns out the patch causes problems when saving queries and thus I wouldn't recommend applying this.

Also available in: Atom PDF