Feature #12898
closed
Handle GET /issues/context_menu parameters nicely to prevent returning error 500 to crawlers
Added by Björn Peemöller over 11 years ago.
Updated over 11 years ago.
Category:
Code cleanup/refactoring
Description
When I navigate to the URL /issues/context_menu in my browser, I get an error 500.
The airbrake notification I get points to line 36 in file app/views/context_menus/issues.html.erb:
ERROR MESSAGE:
NoMethodError: undefined method `include?' for nil:NilClass
WHERE:
context_menus#issues
[PROJECT_ROOT]/app/views/context_menus/issues.html.erb:36
[PROJECT_ROOT]/app/controllers/context_menus_controller.rb:71
URL:
http://<my-domain>/redmine/issues/context_menu
BACKTRACE:
[PROJECT_ROOT]/app/views/context_menus/issues.html.erb:36
[GEM_ROOT]/gems/actionpack-3.2.11/lib/action_view/template.rb:145
[GEM_ROOT]/gems/actionpack-3.2.11/lib/action_view/template.rb:145
...
I do not know in which circumstances this URL is used, but as this error is reproducible by retrieving the URL directly, it might be worth a fix.
My configuration:
Environment:
Redmine version 2.2.1.stable
Ruby version 1.8.7 (i486-linux)
Rails version 3.2.11
Environment production
Database adapter PostgreSQL
Redmine plugins:
redmine_wiki_extensions 0.6.0
- Status changed from New to Closed
- Resolution set to Invalid
This URL is called when right-clicking the issues list and is expecting a ids[]=<issue id> parameter matching each selected issue (try /issues/context_menu?ids[]=10&ids[]=11 or whatever ids).
It is not meant to be called directly.
- Status changed from Closed to Reopened
Okay, I understand that. But I think returning a status code 400 indicating a bad request (or something similar) would be much better than relying on the Ruby code to crash. Therefore, I'm reopening this issue.
Björn Peemöller wrote:
Okay, I understand that. But I think returning a status code 400 indicating a bad request (or something similar) would be much better than relying on the Ruby code to crash. Therefore, I'm reopening this issue.
Why would you care, this URL is intended for internal use only?
Etienne Massip wrote:
Why would you care, this URL is intended for internal use only?
Because I think that the API accessible from the outside world (in this case, accessible URLs) should be stable in the sense that even wrong parameters should not lead to a runtime error.
By the way, I found out that the Google Bot first asked for this URL, which is given in the initialization of the context menu:
<script type="text/javascript">
//<![CDATA[
contextMenuInit('/redmine/issues/context_menu')
//]]>
</script>
I only noticed this because Airbrake automatically notifies me about internal server errors. If you do not intend to change this, I can configure Airbrake to conceal this error, of course.
- Tracker changed from Defect to Feature
- Subject changed from Get-Request of URL /issues/context_menu causes Error 500 to Handle GET /issues/context_menu parameters nicely to prevent returning error 500 to crawlers
- Category set to Code cleanup/refactoring
- Target version set to Candidate for next major release
- Resolution deleted (
Invalid)
Björn Peemöller wrote:
Because I think that the API accessible from the outside world (in this case, accessible URLs) should be stable in the sense that even wrong parameters should not lead to a runtime error.
Agreed but this is not part of an API.
I only noticed this because Airbrake automatically notifies me about internal server errors. If you do not intend to change this, I can configure Airbrake to conceal this error, of course.
No, I simply need a good reason, you just gave me one.
I set it as a FR since it's not exactly a defect to me (it's working as expected), fixer is free to switch it back to Defect.
Thanks, that sounds good.
- Status changed from Reopened to Closed
- Assignee set to Jean-Philippe Lang
- Target version changed from Candidate for next major release to 2.3.0
- Resolution set to Fixed
Fixed in r11226. You get a 404 response now.
Also available in: Atom
PDF
Updated by 
Updated by 
Updated by