Feature #43484 » 0005-Detect-attachment-content-type-from-file-contents-in.patch
| app/assets/javascripts/attachments.js | ||
|---|---|---|
| 122 | 122 |
uploadUrl = uploadUrl + '?attachment_id=' + attachmentId; |
| 123 | 123 |
if (blob instanceof window.Blob) {
|
| 124 | 124 |
uploadUrl += '&filename=' + encodeURIComponent(blob.name); |
| 125 |
uploadUrl += '&content_type=' + encodeURIComponent(blob.type); |
|
| 126 | 125 |
} |
| 127 | 126 | |
| 128 | 127 |
return $.ajax(uploadUrl, {
|
| app/controllers/attachments_controller.rb | ||
|---|---|---|
| 112 | 112 |
@attachment = Attachment.new(:file => raw_request_body) |
| 113 | 113 |
@attachment.author = User.current |
| 114 | 114 |
@attachment.filename = params[:filename].presence || Redmine::Utils.random_hex(16) |
| 115 |
@attachment.content_type = params[:content_type].presence |
|
| 115 |
# The content type is detected from the file contents on save, |
|
| 116 |
# so params[:content_type] is ignored |
|
| 116 | 117 |
saved = @attachment.save |
| 117 | 118 | |
| 118 | 119 |
respond_to do |format| |
| app/models/attachment.rb | ||
|---|---|---|
| 90 | 90 |
File.join(storage_path, 'derived_cache', 'markdownized_previews') |
| 91 | 91 |
end |
| 92 | 92 | |
| 93 |
before_create :files_to_final_location |
|
| 93 |
before_create :set_content_type, :files_to_final_location |
|
| 94 |
before_update :reset_content_type, :if => :filename_changed? |
|
| 94 | 95 |
after_commit :delete_from_disk, :on => :destroy |
| 95 | 96 |
after_commit :reuse_existing_file_if_possible, :on => :create |
| 96 | 97 |
after_rollback :delete_from_disk, :on => :create |
| 97 | 98 | |
| 98 |
safe_attributes 'filename', 'content_type', 'description'
|
|
| 99 |
safe_attributes 'filename', 'description' |
|
| 99 | 100 | |
| 100 | 101 |
# Returns an unsaved copy of the attachment |
| 101 | 102 |
def copy(attributes=nil) |
| ... | ... | |
| 125 | 126 |
self.filename = @temp_file.original_filename |
| 126 | 127 |
self.filename.force_encoding("UTF-8")
|
| 127 | 128 |
end |
| 128 |
if @temp_file.respond_to?(:content_type) |
|
| 129 |
self.content_type = @temp_file.content_type.to_s.chomp |
|
| 130 |
end |
|
| 131 | 129 |
self.filesize = @temp_file.size |
| 132 | 130 |
end |
| 133 | 131 |
end |
| ... | ... | |
| 141 | 139 |
filename |
| 142 | 140 |
end |
| 143 | 141 | |
| 142 |
def set_content_type |
|
| 143 |
return unless @temp_file |
|
| 144 | ||
| 145 |
io = @temp_file.respond_to?(:read) ? @temp_file : StringIO.new(@temp_file) |
|
| 146 |
if io.respond_to?(:rewind) |
|
| 147 |
io.rewind |
|
| 148 |
else |
|
| 149 |
# Marcel rewinds the io while reading it, which a plain reader may not |
|
| 150 |
# support; detect the type from the filename alone in that case |
|
| 151 |
io = nil |
|
| 152 |
end |
|
| 153 |
# The filename is passed as a hint because many formats, plain text and |
|
| 154 |
# Markdown among them, cannot be identified from their contents alone. |
|
| 155 |
# Without it they would all be detected as application/octet-stream. |
|
| 156 |
self.content_type = Marcel::MimeType.for(io, name: filename) |
|
| 157 |
@temp_file.rewind if @temp_file.respond_to?(:rewind) |
|
| 158 |
end |
|
| 159 | ||
| 160 |
# Re-detects the content type from the file on disk. Needed after a rename |
|
| 161 |
# because the filename is used as a hint for contents that cannot be |
|
| 162 |
# identified on their own. |
|
| 163 |
def reset_content_type |
|
| 164 |
return unless readable? |
|
| 165 | ||
| 166 |
File.open(diskfile, 'rb') do |io| |
|
| 167 |
self.content_type = Marcel::MimeType.for(io, name: filename) |
|
| 168 |
end |
|
| 169 |
end |
|
| 170 | ||
| 144 | 171 |
# Copies the temporary file to its final location |
| 145 | 172 |
# and computes its hash |
| 146 | 173 |
def files_to_final_location |
| ... | ... | |
| 164 | 191 |
self.digest = sha.hexdigest |
| 165 | 192 |
end |
| 166 | 193 |
@temp_file = nil |
| 167 | ||
| 168 |
if content_type.blank? && filename.present? |
|
| 169 |
self.content_type = Redmine::MimeType.of(filename) |
|
| 170 |
end |
|
| 171 |
# Don't save the content type if it's longer than the authorized length |
|
| 172 |
if self.content_type && self.content_type.length > 255 |
|
| 173 |
self.content_type = nil |
|
| 174 |
end |
|
| 175 | 194 |
end |
| 176 | 195 | |
| 177 | 196 |
# Deletes the file from the file system if it's not referenced by other attachments |
| app/models/mail_handler.rb | ||
|---|---|---|
| 352 | 352 |
obj.attachments << Attachment.create(:container => obj, |
| 353 | 353 |
:file => attachment.body.decoded, |
| 354 | 354 |
:filename => attachment.filename, |
| 355 |
:author => user, |
|
| 356 |
:content_type => attachment.mime_type) |
|
| 355 |
:author => user) |
|
| 357 | 356 |
end |
| 358 | 357 |
end |
| 359 | 358 |
end |
| lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb | ||
|---|---|---|
| 127 | 127 |
next |
| 128 | 128 |
end |
| 129 | 129 |
a.filename = attachment['filename'] unless attachment['filename'].blank? |
| 130 |
a.content_type = attachment['content_type'] unless attachment['content_type'].blank? |
|
| 131 | 130 |
end |
| 132 | 131 |
next unless a |
| 133 | 132 |
a.description = attachment['description'].to_s.strip |
| test/integration/api_test/attachments_test.rb | ||
|---|---|---|
| 142 | 142 |
assert_nil attachment.container |
| 143 | 143 |
assert_equal 2, attachment.author_id |
| 144 | 144 |
assert_equal 'File content'.size, attachment.filesize |
| 145 |
assert attachment.content_type.blank?
|
|
| 145 |
assert_equal 'application/octet-stream', attachment.content_type
|
|
| 146 | 146 |
assert attachment.filename.present? |
| 147 | 147 |
assert_match %r{\d+_[0-9a-z]+}, attachment.diskfile
|
| 148 | 148 |
assert File.exist?(attachment.diskfile) |
| test/integration/api_test/files_test.rb | ||
|---|---|---|
| 83 | 83 |
assert_response :bad_request |
| 84 | 84 |
end |
| 85 | 85 | |
| 86 |
test "POST /projects/:project_id/files.json should accept :filename, :description, :content_type as optional parameters" do
|
|
| 86 |
test "POST /projects/:project_id/files.json should accept :filename, :description as optional parameters" do |
|
| 87 | 87 |
set_tmp_attachments_directory |
| 88 | 88 |
post( |
| 89 | 89 |
'/uploads.xml', |
| ... | ... | |
| 94 | 94 |
{ "file": {
|
| 95 | 95 |
"filename": "New filename", |
| 96 | 96 |
"description": "New description", |
| 97 |
"content_type": "application/txt", |
|
| 98 | 97 |
"token": "#{token}"
|
| 99 | 98 |
} |
| 100 | 99 |
} |
| ... | ... | |
| 106 | 105 |
assert_response :success |
| 107 | 106 |
assert_equal "New filename", Attachment.last.filename |
| 108 | 107 |
assert_equal "New description", Attachment.last.description |
| 109 |
assert_equal "application/txt", Attachment.last.content_type |
|
| 110 | 108 |
end |
| 111 | 109 | |
| 112 | 110 |
test "POST /projects/:project_id/files.json should accept :version_id to attach the files to a version" do |
| test/integration/attachments_test.rb | ||
|---|---|---|
| 33 | 33 |
assert_equal 'text/plain', attachment.content_type |
| 34 | 34 |
end |
| 35 | 35 | |
| 36 |
def test_upload_should_accept_content_type_param
|
|
| 36 |
def test_upload_should_detect_file_content_type
|
|
| 37 | 37 |
log_user('jsmith', 'jsmith')
|
| 38 | 38 |
assert_difference 'Attachment.count' do |
| 39 | 39 |
post( |
| 40 |
"/uploads.js?attachment_id=1&filename=foo&content_type=image/jpeg",
|
|
| 41 |
:params => "File content",
|
|
| 40 |
"/uploads.js?attachment_id=1&filename=foo", |
|
| 41 |
:params => "\xCA\xFE\xBA\xBE", # Java class file magic bytes
|
|
| 42 | 42 |
:headers => {"CONTENT_TYPE" => 'application/octet-stream'})
|
| 43 | 43 |
assert_response :success |
| 44 | 44 |
end |
| 45 | 45 |
attachment = Attachment.order(:id => :desc).first |
| 46 |
assert_equal 'image/jpeg', attachment.content_type
|
|
| 46 |
assert_equal 'application/java-vm', attachment.content_type
|
|
| 47 | 47 |
end |
| 48 | 48 | |
| 49 | 49 |
def test_upload_as_js_and_attach_to_an_issue |
| test/unit/attachment_test.rb | ||
|---|---|---|
| 59 | 59 |
assert_equal 59, File.size(a.diskfile) |
| 60 | 60 |
end |
| 61 | 61 | |
| 62 |
def test_create_should_clear_content_type_if_too_long |
|
| 62 |
def test_create_should_not_trust_client_declared_content_type |
|
| 63 |
# PDF file uploaded with a wrong content type "text/plain" |
|
| 64 |
# and a long bogus content_type attribute |
|
| 63 | 65 |
a = Attachment.new(:container => Issue.find(1), |
| 64 |
:file => uploaded_test_file("testfile.txt", "text/plain"),
|
|
| 66 |
:file => uploaded_test_file("hello.pdf", "text/plain"),
|
|
| 65 | 67 |
:author => User.find(1), |
| 66 | 68 |
:content_type => 'a'*300) |
| 67 | 69 |
assert a.save |
| 68 | 70 |
a.reload |
| 69 |
assert_nil a.content_type |
|
| 71 |
assert_equal 'application/pdf', a.content_type |
|
| 72 |
end |
|
| 73 | ||
| 74 |
def test_create_should_accept_a_file_that_cannot_be_rewound |
|
| 75 |
# A minimal reader like the ones plugins may pass to Attachment#file= |
|
| 76 |
reader = Class.new do |
|
| 77 |
def initialize(content) |
|
| 78 |
@content = content |
|
| 79 |
end |
|
| 80 | ||
| 81 |
def size |
|
| 82 |
@content.bytesize |
|
| 83 |
end |
|
| 84 | ||
| 85 |
def read(*args) |
|
| 86 |
if @eof |
|
| 87 |
false |
|
| 88 |
else |
|
| 89 |
@eof = true |
|
| 90 |
@content |
|
| 91 |
end |
|
| 92 |
end |
|
| 93 |
end.new("hello world\n")
|
|
| 94 | ||
| 95 |
a = Attachment.new(:file => reader, :filename => 'note.txt', :author_id => 1) |
|
| 96 |
assert a.save |
|
| 97 |
# The contents cannot be inspected without consuming the reader, |
|
| 98 |
# so the content type is detected from the filename |
|
| 99 |
assert_equal 'text/plain', a.content_type |
|
| 100 |
assert_equal "hello world\n", File.read(a.diskfile) |
|
| 101 |
end |
|
| 102 | ||
| 103 |
def test_rename_should_recalculate_the_content_type |
|
| 104 |
a = Attachment.create!( |
|
| 105 |
:file => mock_file_with_options(:original_filename => 'test.bin', :content => "\x00\x01\x02".b), |
|
| 106 |
:author_id => 1 |
|
| 107 |
) |
|
| 108 |
assert_equal 'application/octet-stream', a.content_type |
|
| 109 | ||
| 110 |
assert a.update(:filename => 'renamed.txt') |
|
| 111 |
assert_equal 'text/plain', a.reload.content_type |
|
| 112 |
end |
|
| 113 | ||
| 114 |
def test_rename_should_not_fail_when_the_file_is_missing |
|
| 115 |
a = Attachment.create!( |
|
| 116 |
:file => mock_file_with_options(:original_filename => 'test.bin', :content => "\x00\x01\x02".b), |
|
| 117 |
:author_id => 1 |
|
| 118 |
) |
|
| 119 |
File.delete(a.diskfile) |
|
| 120 | ||
| 121 |
assert a.update(:filename => 'renamed.txt') |
|
| 70 | 122 |
end |
| 71 | 123 | |
| 72 | 124 |
def test_shorted_filename_if_too_long |