Commit 6475a4d3a3bef0a553b844b13068b077e78f3146
Committed by
Rodrigo Souto
1 parent
bb031599
Exists in
master
and in
22 other branches
Force a browser to save file as after clicking a uploaded file link
(ActionItem2829)
Showing
5 changed files
with
36 additions
and
6 deletions
Show diff stats
app/controllers/public/content_viewer_controller.rb
| @@ -53,8 +53,9 @@ class ContentViewerController < ApplicationController | @@ -53,8 +53,9 @@ class ContentViewerController < ApplicationController | ||
| 53 | # At this point the page will be showed | 53 | # At this point the page will be showed |
| 54 | @page.hit | 54 | @page.hit |
| 55 | 55 | ||
| 56 | - unless @page.mime_type == 'text/html' || (@page.image? && params[:view]) | 56 | + if @page.download? params[:view] |
| 57 | headers['Content-Type'] = @page.mime_type | 57 | headers['Content-Type'] = @page.mime_type |
| 58 | + headers.merge! @page.download_headers | ||
| 58 | data = @page.data | 59 | data = @page.data |
| 59 | 60 | ||
| 60 | # TODO test the condition | 61 | # TODO test the condition |
| @@ -70,7 +71,7 @@ class ContentViewerController < ApplicationController | @@ -70,7 +71,7 @@ class ContentViewerController < ApplicationController | ||
| 70 | 71 | ||
| 71 | #FIXME see a better way to do this. It's not need to pass this variable anymore | 72 | #FIXME see a better way to do this. It's not need to pass this variable anymore |
| 72 | @comment = Comment.new | 73 | @comment = Comment.new |
| 73 | - | 74 | + |
| 74 | if @page.has_posts? | 75 | if @page.has_posts? |
| 75 | posts = if params[:year] and params[:month] | 76 | posts = if params[:year] and params[:month] |
| 76 | filter_date = DateTime.parse("#{params[:year]}-#{params[:month]}-01") | 77 | filter_date = DateTime.parse("#{params[:year]}-#{params[:month]}-01") |
app/models/article.rb
| @@ -188,7 +188,7 @@ class Article < ActiveRecord::Base | @@ -188,7 +188,7 @@ class Article < ActiveRecord::Base | ||
| 188 | pending_categorizations.clear | 188 | pending_categorizations.clear |
| 189 | end | 189 | end |
| 190 | 190 | ||
| 191 | - acts_as_taggable | 191 | + acts_as_taggable |
| 192 | N_('Tag list') | 192 | N_('Tag list') |
| 193 | 193 | ||
| 194 | acts_as_filesystem | 194 | acts_as_filesystem |
| @@ -268,7 +268,7 @@ class Article < ActiveRecord::Base | @@ -268,7 +268,7 @@ class Article < ActiveRecord::Base | ||
| 268 | end | 268 | end |
| 269 | 269 | ||
| 270 | # returns the data of the article. Must be overriden in each subclass to | 270 | # returns the data of the article. Must be overriden in each subclass to |
| 271 | - # provide the correct content for the article. | 271 | + # provide the correct content for the article. |
| 272 | def data | 272 | def data |
| 273 | body | 273 | body |
| 274 | end | 274 | end |
| @@ -360,6 +360,16 @@ class Article < ActiveRecord::Base | @@ -360,6 +360,16 @@ class Article < ActiveRecord::Base | ||
| 360 | false | 360 | false |
| 361 | end | 361 | end |
| 362 | 362 | ||
| 363 | + def download? view = nil | ||
| 364 | + (self.uploaded_file? and not self.image?) or | ||
| 365 | + (self.image? and view.blank?) or | ||
| 366 | + (not self.uploaded_file? and self.mime_type != 'text/html') | ||
| 367 | + end | ||
| 368 | + | ||
| 369 | + def download_headers | ||
| 370 | + {} | ||
| 371 | + end | ||
| 372 | + | ||
| 363 | named_scope :native_translations, :conditions => { :translation_of_id => nil } | 373 | named_scope :native_translations, :conditions => { :translation_of_id => nil } |
| 364 | 374 | ||
| 365 | def translatable? | 375 | def translatable? |
app/models/uploaded_file.rb
| @@ -86,6 +86,12 @@ class UploadedFile < Article | @@ -86,6 +86,12 @@ class UploadedFile < Article | ||
| 86 | self.name = self.filename | 86 | self.name = self.filename |
| 87 | end | 87 | end |
| 88 | 88 | ||
| 89 | + def download_headers | ||
| 90 | + { | ||
| 91 | + 'Content-Disposition' => "attachment; filename=\"#{self.filename}\"", | ||
| 92 | + } | ||
| 93 | + end | ||
| 94 | + | ||
| 89 | def data | 95 | def data |
| 90 | File.read(self.full_filename) | 96 | File.read(self.full_filename) |
| 91 | end | 97 | end |
test/functional/content_viewer_controller_test.rb
| @@ -64,7 +64,19 @@ class ContentViewerControllerTest < ActionController::TestCase | @@ -64,7 +64,19 @@ class ContentViewerControllerTest < ActionController::TestCase | ||
| 64 | assert_response :missing | 64 | assert_response :missing |
| 65 | end | 65 | end |
| 66 | 66 | ||
| 67 | - should 'produce a download-like when article is not text/html' do | 67 | + should 'produce a download-link when article is a uploaded file' do |
| 68 | + profile = create_user('someone').person | ||
| 69 | + html = UploadedFile.create! :uploaded_data => fixture_file_upload('/files/500.html', 'text/html'), :profile => profile | ||
| 70 | + html.save! | ||
| 71 | + | ||
| 72 | + get :view_page, :profile => 'someone', :page => [ '500.html' ] | ||
| 73 | + | ||
| 74 | + assert_response :success | ||
| 75 | + assert_match /^text\/html/, @response.headers['Content-Type'] | ||
| 76 | + assert @response.headers['Content-Disposition'].present? | ||
| 77 | + end | ||
| 78 | + | ||
| 79 | + should 'produce a download-link when article is not text/html' do | ||
| 68 | 80 | ||
| 69 | # for example, RSS feeds | 81 | # for example, RSS feeds |
| 70 | profile = create_user('someone').person | 82 | profile = create_user('someone').person |
| @@ -1254,7 +1266,7 @@ class ContentViewerControllerTest < ActionController::TestCase | @@ -1254,7 +1266,7 @@ class ContentViewerControllerTest < ActionController::TestCase | ||
| 1254 | 1266 | ||
| 1255 | get 'view_page', :profile => profile.identifier, :page => article.path.split('/') | 1267 | get 'view_page', :profile => profile.identifier, :page => article.path.split('/') |
| 1256 | assert_tag :tag => 'a', :attributes => { :href => "/#{profile.identifier}/#{article.path}?comment_page=2", :rel => 'next' } | 1268 | assert_tag :tag => 'a', :attributes => { :href => "/#{profile.identifier}/#{article.path}?comment_page=2", :rel => 'next' } |
| 1257 | - end | 1269 | + end |
| 1258 | 1270 | ||
| 1259 | should 'not escape acceptable HTML in list of blog posts' do | 1271 | should 'not escape acceptable HTML in list of blog posts' do |
| 1260 | login_as('testinguser') | 1272 | login_as('testinguser') |