1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Commit graph

96 commits

Author SHA1 Message Date
George Claghorn
d5a2f7ec14
Mirror direct uploads 2019-05-22 15:07:35 -04:00
George Claghorn
a89e9cccb2
Merge pull request from yfxie/fix-normalize-the-hash-of-transformations
ActiveStorage - normalize the hash of transformations
2019-04-21 16:07:54 -04:00
Younes SERRAJ
bcf370d689 Allow ActiveStorage to generate variants of BMP images 2019-04-21 19:07:22 +02:00
Yi Feng
6be1446fc7 normalize the hash of transformations 2019-04-20 18:05:58 +08:00
Sharang Dashputre
771973c13d url -> URL where apt except inside actionpack/ 2019-04-01 22:56:35 +05:30
George Claghorn
562f3a2d21 Add ActiveStorage::Service#open 2019-03-28 18:47:42 -04:00
Abhishek Chandrasekhar
32438ed64f [ActiveStorage] Ensure that the _blob association is properly loaded when attaching ::One
Consider a model with `One` and `Many` attachments configured:

    class User < ActiveRecord::Base
      has_one_attached :avatar
      has_many_attached :highlights
    end

=== One Attachment

After attaching `One` attachment (`:avatar`), we can see that the associated
`_blob` record (`:avatar_blob`) still returns as `nil`.

    user.avatar.attach(blob)
    user.avatar_attachment.present?  => true
    user.avatar_blob.present?        => false    # Incorrect!

This is a false negative. It happens because after the attachment and blob
are built:

  1. The record already has its `_blob` association loaded, as `nil`
  2. the `::Attachment` is associated with the record but the `::Blob` only gets
    associated with the `::Attachment`, not the record itself

In reality, the blob does in fact exist. We can verify this as follows:

    user.avatar.attach(blob)
    user.avatar_attachment.blob.present?    => true  # Blob does exist!

The fix in this change is to simply assign the `::Blob` when assigning
the `::Attachment`. After this fix is applied, we correctly observe:

    user.avatar.attach(blob)
    user.avatar_attachment.present?  => true
    user.avatar_blob.present?        => true    # Woohoo!

=== Many Attachments

We don't see this issue with `Many` attachments because the `_blob` association
is already loaded as part of attaching more/newer blobs.

    user.highlights.attach(blob)
    user.highlights_attachments.any?    => true
    user.highlights_blobs.any?          => true
2019-02-26 11:10:38 -05:00
Luciano Sousa
c329d323fc Permit generating variants of TIFF images 2018-12-30 18:14:49 -05:00
Julik Tarkhanov
e5f4162b61 Make Active Storage blob keys lowercase
Accommodate case-insensitive filesystems and database collations.
2018-12-30 11:56:22 -05:00
Ryuta Kamizono
892e38c78e Enable Style/RedundantBegin cop to avoid newly adding redundant begin block
Currently we sometimes find a redundant begin block in code review
(e.g. https://github.com/rails/rails/pull/33604#discussion_r209784205).

I'd like to enable `Style/RedundantBegin` cop to avoid that, since
rescue/else/ensure are allowed inside do/end blocks in Ruby 2.5
(https://bugs.ruby-lang.org/issues/12906), so we'd probably meets with
that situation than before.
2018-12-21 06:12:42 +09:00
Ryuta Kamizono
8034dde023 Module#{define_method,alias_method,undef_method,remove_method} become public since Ruby 2.5
https://bugs.ruby-lang.org/issues/14133
2018-12-21 01:39:18 +09:00
yuuji.yaginuma
c2ef8bbf52 Fix broken ActiveStorage::BlobTest
`ActiveStorage::Filename#parameters` was removed by .
2018-11-28 10:16:07 +09:00
Rosa Gutierrez
06ab7b27ea Prevent content type and disposition bypass in storage service URLs
* Force content-type to binary on service urls for relevant content types

We have a list of content types that must be forcibly served as binary,
but in practice this only means to serve them as attachment always. We
should also set the Content-Type to the configured binary type.

As a bonus: add text/cache-manifest to the list of content types to be
served as binary by default.

* Store content-disposition and content-type in GCS

Forcing these in the service_url when serving the file works fine for S3
and Azure, since these services include params in the signature.
However, GCS specifically excludes response-content-disposition and
response-content-type from the signature, which means an attacker can
modify these and have files that should be served as text/plain attachments
served as inline HTML for example. This makes our attempt to force
specific files to be served as binary and as attachment can be easily
bypassed.

The only way this can be forced in GCS is by storing
content-disposition and content-type in the object metadata.

* Update GCS object metadata after identifying blob

In some cases we create the blob and upload the data before identifying
the content-type, which means we can't store that in GCS right when
uploading. In these, after creating the attachment, we enqueue a job to
identify the blob, and set the content-type.

In other cases, files are uploaded to the storage service via direct
upload link. We create the blob before the direct upload, which happens
independently from the blob creation itself. We then mark the blob as
identified, but we have already the content-type we need without having
put it in the service.

In these two cases, then, we need to update the metadata in the GCS
service.

* Include content-type and disposition in the verified key for disk service

This prevents an attacker from modifying these params in the service
signed URL, which is particularly important when we want to force them
to have specific values for security reasons.

* Allow only a list of specific content types to be served inline

This is different from the content types that must be served as binary
in the sense that any content type not in this list will be always
served as attachment but with its original content type. Only types in
this list are allowed to be served either inline or as attachment.

Apart from forcing this in the service URL, for GCS we need to store the
disposition in the metadata.

Fix CVE-2018-16477.
2018-11-27 15:36:27 -05:00
Kasper Timm Hansen
ed56a03104
Merge pull request from mtsmfm/encode-filename
Encode Content-Disposition filenames on send_data and send_file
2018-09-23 19:43:06 +02:00
yuuji.yaginuma
1b86d90136 Enable Performance/UnfreezeString cop
In Ruby 2.3 or later, `String#+@` is available and `+@` is faster than `dup`.

```ruby
# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "benchmark-ips"
end

Benchmark.ips do |x|
  x.report('+@') { +"" }
  x.report('dup') { "".dup }
  x.compare!
end
```

```
$ ruby -v benchmark.rb
ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux]
Warming up --------------------------------------
                  +@   282.289k i/100ms
                 dup   187.638k i/100ms
Calculating -------------------------------------
                  +@      6.775M (± 3.6%) i/s -     33.875M in   5.006253s
                 dup      3.320M (± 2.2%) i/s -     16.700M in   5.032125s

Comparison:
                  +@:  6775299.3 i/s
                 dup:  3320400.7 i/s - 2.04x  slower

```
2018-09-23 08:56:55 +09:00
Fumiaki MATSUSHIMA
890485cfce Encode Content-Disposition filenames on send_data and send_file 2018-09-13 21:38:46 +09:00
Jasper Martin
934fccd522 Ignore ActiveRecord::InvalidForeignKey in ActiveStorage::Blob#purge
Do nothing instead of raising an error when it’s called on an attached blob.
2018-07-26 09:24:31 -04:00
George Claghorn
562ec3dcd1 Test that ActiveStorage::Blob#purge fails when attachments exist 2018-07-20 10:28:14 -04:00
George Claghorn
2ae3a29508 Add a foreign-key constraint to the attachments table for blobs 2018-07-19 20:43:33 -04:00
George Claghorn
af02b9b78f Remove unnecessary tap 2018-07-17 16:23:53 -04:00
George Claghorn
e13e16f5ad Fix replacing many attachments via assign and attach 2018-07-17 09:33:48 -04:00
George Claghorn
379d98dcd4 Correct test name 2018-07-16 15:59:17 -04:00
George Claghorn
36ec5428bf Fix that successive ActiveStorage::Attached::Many#attach calls would overwrite previous attachments 2018-07-16 15:57:43 -04:00
George Claghorn
1d13de4e39 Test removing attachments via #attach 2018-07-16 08:59:23 -04:00
George Claghorn
bd5eba1adf Clear attachment changes on reload 2018-07-13 14:48:45 -04:00
George Claghorn
28db8ba60e Implement ActiveStorage::Attached::{One,Many}#attach in terms of changes 2018-07-13 13:29:33 -04:00
George Claghorn
d20d6c7326 Fix that detaching could purge 2018-07-13 12:17:33 -04:00
George Claghorn
63951072af Fix analyzing new blobs from uploaded files on attach 2018-07-13 06:33:45 -04:00
George Claghorn
0b85123cd8 Raise an ArgumentError instead of a RuntimeError 2018-07-08 11:35:27 -04:00
George Claghorn
e8682c5bf0
Store newly-uploaded files on save rather than assignment 2018-07-07 23:25:33 -04:00
George Claghorn
03afddd2eb Fix that models can clobber each others' attachment reflections
Consider the following model definitions:

    class User < ApplicationRecord
      has_one_attached :avatar
    end

    class Group < ApplicationRecord
      has_one_attached :avatar
    end

If you attempt to reflect on the User model's avatar attachment via User.reflect_on_attachment, you could receive a reflection for the Group model's avatar attachment. Fix this by ensuring that each model class uses its own Hash object to track attachment reflections.
2018-07-07 17:09:31 -04:00
George Claghorn
b21f50d8ae Permit configuring the default service URL expiry 2018-06-21 11:06:32 -04:00
bogdanvlviv
8bc9062aee
Refactor activestorage/test/models/attached_test.rb
Don't include `ActiveJob::TestHelper` since there is no test that uses it.

Ensure removing of overridden User's methods.

Related to https://github.com/rails/rails/pull/33085#issuecomment-395548563
Module#remove_method is private in Ruby 2.4.

Related to fd0bd1bf68
2018-06-07 23:29:03 +03:00
Rafael França
c1844477a1
Merge pull request from kddeisz/defined-attachments
ActiveStorage reflection
2018-06-01 14:24:06 -04:00
Kevin Deisz
ce337d1757
Move ActiveStorage reflection logic entirely into ActiveStorage 2018-05-31 09:33:46 -04:00
George Claghorn
bd7ebf61fb Remove errant debugger call 2018-05-30 20:09:30 -04:00
George Claghorn
a6d80e164f Include blob ID in tempfile name for debugging convenience 2018-05-30 20:05:39 -04:00
Kevin Deisz
bc3b6ea461
Reflection for attachments
Add the ability to reflect on the attachments that have been defined using ActiveRecord::Reflection.
2018-05-30 13:28:22 -04:00
George Claghorn
1bdaccc0b8 Verify integrity after chunked download 2018-05-28 16:28:46 -04:00
Javan Makhmali
b60ee86d94 Change video preview format from PNG to JPG 2018-05-23 14:32:34 -04:00
Jacob Smith
0210ac0b43 Disable variant options when false or nil present
In response to https://github.com/rails/rails/issues/32917

In the current implementation, ActiveStorage passes all options to the underlying processor,
including when a key has a value of false.

For example, passing:

```
avatar.variant(resize: "100x100", monochrome: false, flip: "-90")
```

will return a monochrome image (or an error, pending on ImageMagick configuration) because
it passes `-monochrome false` to the command (but the command line does not allow disabling
flags this way, as usually a user would omit the flag entirely to disable that feature).

This fix only passes those keys forward to the underlying processor if the value responds to
`present?`. In practice, this means that `false` or `nil` will be filtered out before going
to the processor.

One possible use case would be for a user to be able to apply different filters to an avatar.
The code might look something like:

```
  variant_options = {
    monochrome: params[:monochrome],
    resize:     params[:resize]
  }

  avatar.variant(*variant_options)
```

Obviously some sanitization may be beneficial in a real-world scenario, but this type of
configuration object could be used in many other places as well.

- Add removing falsy values from varaints to changelog

- The entirety of #image_processing_transformation inject block was wrapped in `list.tap`
 to guard against the default `nil` being returned if no conditional was called.

- add test for explicitly true variant options
2018-05-21 10:38:15 -04:00
George Claghorn
9f95767979 Permit opening a blob in a custom tempdir 2018-05-17 19:14:11 -04:00
Josh Susser
fd0bd1bf68 Generate getter and setter methods in mixin
Generated attachment getter and setter methods are created within
the model's `GeneratedAssociationMethods` module to allow overriding
and composition using `super`.

Includes tests for new functionality.

Co-authored-by: Josh Susser <josh@hasmanythrough.com>
Co-authored-by: Jamon Douglas <terrildouglas@gmail.com>
2018-05-17 14:51:15 -07:00
George Claghorn
ee21b7c2eb Add ActiveStorage::Blob#open
[David Robertson & George Claghorn]
2018-05-16 22:12:31 -04:00
Ryan Davidson
8e98bb7758 Add option to ActiveStorage::Blob to set extract_content_type_from_io
This adds a boolean argument called identify to ActiveStorage::Blob
methods #create_after_upload, #build_after_upload and #upload. It
allows a user to bypass the automatic content_type inference from
the io.
2018-05-08 23:01:57 +01:00
George Claghorn
5f2ee4c0bb Stream blobs from disk in 5 MB chunks
Match other services, which all use a 5 MB chunk size.
2018-04-29 07:07:59 -04:00
Eileen M. Uchitelle
ad0220a71a
Merge pull request from fatkodima/has_attached-presence-validation
has_(one/many)_attached presence validation
2018-04-27 14:35:21 -04:00
Janko Marohnić
7fc8b6d82c
Show ImageProcessing macros in a dedicated example 2018-04-23 12:21:42 +02:00
Janko Marohnić
f01e249890
Rename ActiveStorage.processor to .variant_processor 2018-04-22 23:40:42 +02:00
Janko Marohnić
ca12968587
Use ImageProcessing gem for ActiveStorage variants
ImageProcessing gem is a wrapper around MiniMagick and ruby-vips, and
implements an interface for common image resizing and processing. This
is the canonical image processing gem recommended in [Shrine], and
that's where it developed from. The initial implementation was extracted
from Refile, which also implements on-the-fly transformations.

Some features that ImageProcessing gem adds on top of MiniMagick:

  * resizing macros
    - #resize_to_limit
    - #resize_to_fit
    - #resize_to_fill
    - #resize_and_pad
  * automatic orientation
  * automatic thumbnail sharpening
  * avoids the complex and inefficient MiniMagick::Image class
  * will use "magick" instead of "convert" on ImageMagick 7

However, the biggest feature of the ImageProcessing gem is that it has
an alternative implementation that uses libvips. Libvips is an
alternative to ImageMagick that can process images very rapidly (we've
seen up 10x faster than ImageMagick).

What's great is that the ImageProcessing gem provides the same interface
for both implementations. The macros are named the same, and the libvips
implementation does auto orientation and thumbnail sharpening as well;
only the operations/options specific to ImageMagick/libvips differ. The
integration provided by this PR should work for both implementations.

The plan is to introduce the ImageProcessing backend in Rails 6.0 as the
default backend and deprecate the MiniMagick backend, then in Rails 6.1
remove the MiniMagick backend.
2018-04-18 17:46:25 +02:00