Commit Graph

245 Commits

Author SHA1 Message Date
Sam
f96ef33856
FIX: dominant color not working for 16bit images (#20300)
16 bit images were not returning the correct dominant color due truncation

The routine expected an 8bit color eg: #FFAA00, but ended up getting a 16bit one eg: #FFFAAA000. This caused a truncation, which leads to wildly off colors.
2023-02-15 12:41:04 +11:00
Martin Brennan
0924f874bd
DEV: Use UploadReference instead of ChatUpload in chat (#19947)
We've had the UploadReference table for some time now in core,
but it was added after ChatUpload was and chat was just never
moved over to this new system.

This commit changes all chat code dealing with uploads to create/
update/delete/query UploadReference records instead of ChatUpload
records for consistency. At a later date we will drop the ChatUpload
table, but for now keeping it for data backup.

The migration + post migration are the same, we need both in case
any chat uploads are added/removed during deploy.
2023-01-24 13:28:21 +10:00
Daniel Waterworth
666536cbd1
DEV: Prefer \A and \z over ^ and $ in regexes (#19936) 2023-01-20 12:52:49 -06:00
David Taylor
5a003715d3
DEV: Apply syntax_tree formatting to app/* 2023-01-09 14:14:59 +00:00
Jarek Radosz
dc8a7e74f4
FIX: Allow attr updates of over-size-limit uploads (#18986) 2022-11-11 17:56:11 +01:00
Daniel Waterworth
167181f4b7
DEV: Quote values when constructing SQL (#18827)
All of these cases should already be safe, but still good to quote for
"defense in depth".
2022-11-01 14:05:13 -05:00
David Taylor
76c86a4269
FIX: Correctly handle HTTP errors during dominant color calculation (#18565)
The previous fix in e83d35d6 was incorrect, and the stub in the test was never actually hit. This commit moves the error handling to the right place and updates the specs to ensure the stub is always used.
2022-10-12 15:50:44 +01:00
David Taylor
e83d35d6f3
FIX: Improve error handling for calculate_dominant_color! (#18503)
These errors tend to indicate that the upload is missing on the remote store. This is bad, but we don't want it to block the dominant-color calculation process. This commit catches errors when there is an HTTP error, and fixes the `base_store.rb` implementation when `FileHelper.download` returns nil.
2022-10-06 13:44:53 +01:00
Martin Brennan
8ebd5edd1e
DEV: Rename secure_media to secure_uploads (#18376)
This commit renames all secure_media related settings to secure_uploads_* along with the associated functionality.

This is being done because "media" does not really cover it, we aren't just doing this for images and videos etc. but for all uploads in the site.

Additionally, in future we want to secure more types of uploads, and enable a kind of "mixed mode" where some uploads are secure and some are not, so keeping media in the name is just confusing.

This also keeps compatibility with the `secure-media-uploads` path, and changes new
secure URLs to be `secure-uploads`.

Deprecated settings:

* secure_media -> secure_uploads
* secure_media_allow_embed_images_in_emails -> secure_uploads_allow_embed_images_in_emails
* secure_media_max_email_embed_image_size_kb -> secure_uploads_max_email_embed_image_size_kb
2022-09-29 09:24:33 +10:00
David Taylor
42947ec6f1
FIX: Handle failed download when calculating image dominant color (#18342)
This can happen when the upload size exceeds the maximum upload size, or there is a network issue during download
2022-09-23 12:42:07 +01:00
David Taylor
0f5a8cc526
DEV: Enforce dominant_color length in validation (#18309)
The `add_column` `limit` parameter has no effect on a postgres `text` column. Instead we can perform the check in ActiveRecord.

We never expect this condition to be hit - users cannot control this value. It's just a safety net.
2022-09-21 11:01:21 +01:00
David Taylor
d0243f741e
UX: Use dominant color as image loading placeholder (#18248)
We previously had a system which would generate a 10x10px preview of images and add their URLs in a data-small-upload attribute. The client would then use that as the background-image of the `<img>` element. This works reasonably well on fast connections, but on slower connections it can take a few seconds for the placeholders to appear. The act of loading the placeholders can also break or delay the loading of the 'real' images.

This commit replaces the placeholder logic with a new approach. Instead of a 10x10px preview, we use imagemagick to calculate the average color of an image and store it in the database. The hex color value then added as a `data-dominant-color` attribute on the `<img>` element, and the client can use this as a `background-color` on the element while the real image is loading. That means no extra HTTP request is required, and so the placeholder color can appear instantly.

Dominant color will be calculated:
1. When a new upload is created
2. During a post rebake, if the dominant color is missing from an upload, it will be calculated and stored
3. Every 15 minutes, 25 old upload records are fetched and their dominant color calculated and stored. (part of the existing PeriodicalUpdates job)

Existing posts will continue to use the old 10x10px placeholder system until they are next rebaked
2022-09-20 10:28:17 +01:00
David Taylor
6650218e3d
FIX: Ensure that extract_upload_ids works with all short URLs (#17070)
We do not zero-pad our base62 short URLs, so there is no guarantee that the length is 27. Instead, let's greedily match all consecutive base62 characters and look for a matching upload.

This reverts bd32656157 and 36f5d5eada.
2022-06-13 17:01:27 +01:00
Bianca Nenciu
9db8f00b3d
FEATURE: Create upload_references table (#16146)
This table holds associations between uploads and other models. This can be used to prevent removing uploads that are still in use.

* DEV: Create upload_references
* DEV: Use UploadReference instead of PostUpload
* DEV: Use UploadReference for SiteSetting
* DEV: Use UploadReference for Badge
* DEV: Use UploadReference for Category
* DEV: Use UploadReference for CustomEmoji
* DEV: Use UploadReference for Group
* DEV: Use UploadReference for ThemeField
* DEV: Use UploadReference for ThemeSetting
* DEV: Use UploadReference for User
* DEV: Use UploadReference for UserAvatar
* DEV: Use UploadReference for UserExport
* DEV: Use UploadReference for UserProfile
* DEV: Add method to extract uploads from raw text
* DEV: Use UploadReference for Draft
* DEV: Use UploadReference for ReviewableQueuedPost
* DEV: Use UploadReference for UserProfile's bio_raw
* DEV: Do not copy user uploads to upload references
* DEV: Copy post uploads again after deploy
* DEV: Use created_at and updated_at from uploads table
* FIX: Check if upload site setting is empty
* DEV: Copy user uploads to upload references
* DEV: Make upload extraction less strict
2022-06-09 09:24:30 +10:00
Martin Brennan
48481dd6ed
DEV: Remove ignored columns (#16645)
Bookmark columns deleted in b22450c7a8
TopicTimer columns deleted in d098f51ad3
Upload columns deleted in ef90575b91
2022-05-05 12:22:17 +10:00
David Taylor
c1db968740
DEV: Move hotlinked image information into a dedicated table (#16585)
This will make future changes to the 'pull hotlinked images' system easier. This commit should not introduce any functional change.

For now, the old post_custom_field data is kept in the database. This will be dropped in a future commit.
2022-05-03 13:53:32 +01:00
Sam
cedcdb0057
FEATURE: allow for local theme js assets (#16374)
Due to default CSP web workers instantiated from CDN based assets are still
treated as "same-origin" meaning that we had no way of safely instansiating
a web worker from a theme.

This limits the theme system and adds the arbitrary restriction that WASM
based components can not be safely used.

To resolve this limitation all js assets in about.json are also cached on
local domain.

{
  "name": "Header Icons",
  "assets" : {
    "worker" : "assets/worker.js"
  }
}

This can then be referenced in JS via:

settings.theme_uploads_local.worker

local_js_assets are unconditionally served from the site directly and
bypass the entire CDN, using the pre-existing JavascriptCache

Previous to this change this code was completely dormant on sites which
used s3 based uploads, this reuses the very well tested and cached asset
system on s3 based sites.

Note, when creating local_js_assets it is highly recommended to keep the
assets lean and keep all the heavy working in CDN based assets. For example
wasm files can still live on the CDN but the lean worker that loads it can
live on local.

This change unlocks wasm in theme components, so wasm is now also allowed
in `theme_authorized_extensions`

* more usages of upload.content

* add a specific test for upload.content

* Adjust logic to ensure that after upgrades we still get a cached local js
on save
2022-04-07 07:58:10 +10:00
Bianca Nenciu
5eaf214594
FEATURE: New plugin API to check if upload is used (#15545)
This commit introduces two new APIs for handling unused uploads, one
can be used to exclude uploads in bulk when the data model allow and
the other one excludes uploads one by one.
2022-02-16 09:00:30 +02:00
Alan Guo Xiang Tan
6fb89c153a Revert "DEV: Remove stale ignored_columns from models."
This reverts commit 9f5c8644d0.

Have to revert because the ignored columns have not been dropped.
2022-01-11 11:00:58 +08:00
Alan Guo Xiang Tan
9f5c8644d0 DEV: Remove stale ignored_columns from models. 2022-01-11 10:38:10 +08:00
Vinoth Kannan
a6de4a5ce9
DEV: use upload id to save in theme setting instead of URL. (#14341)
When we use URL instead it creates the problem while changing the CDN hostname.
2021-09-16 07:58:53 +05:30
Martin Brennan
581482003a
DEV: Change uploads.filesize column to bigint (#14334)
This is necessary to allow for large file uploads via
the direct S3 upload mechanism, as we convert the external
file to an Upload record via ExternalUploadManager once
it is complete.

This will allow for files larger than 2,147,483,647 bytes (2.14GB)
to be referenced in the uploads table.

This is a table locking migration, but since it is not as highly
trafficked as posts, topics, or users, the disruption should be minimal.
2021-09-14 12:20:56 +10:00
Martin Brennan
9f275c12ab
FIX: Handle storage providers not implementing ACLs (#13675)
When secure media is enabled or when upload secure status
is updated, we also try and update the upload ACL. However
if the object storage provider does not implement this we
get an Aws::S3::Errors::NotImplemented error. This PR handles
this error so the update_secure_status method does not error
out and still returns whether the secure status changed.
2021-07-09 11:31:44 +10:00
Jarek Radosz
046a875222
DEV: Improve script/downsize_uploads.rb (#13508)
* Only shrink images that are used in Posts and no other models
* Don't save the upload if the size is the same
2021-06-24 00:09:40 +02:00
Sam
5deda5ef3e
FIX: automatically timeout long running image magick commands (#12670)
Previously certain images may lead to convert / identify to run for unreasonable
amounts of time

This adds a maximum amount of time these commands can run prior to forcing
them to stop
2021-04-12 13:55:54 +10:00
jbrw
a9b6f4d829
FIX - use ImageMagick to determine size of svg images (#12230)
SVG files can have dimensions expressed in inches, centimeters, etc., which may lead to the dimensions being misinterpreted (e.g. “8in” ends up as 8 pixels).

If the file type is `svg`, ask ImageMagick to work out what size the SVG file should be rendered on screen.

NOTE: The `pencil.svg` file was obtained from https://freesvg.org/1534028868, which has placed the file in to the public domain.
2021-03-01 11:44:00 -05:00
Martin Brennan
f49e3e5731
DEV: Add security_last_changed_at and security_last_changed_reason to uploads (#11860)
This PR adds security_last_changed_at and security_last_changed_reason to uploads. This has been done to make it easier to track down why an upload's secure column has changed and when. This necessitated a refactor of the UploadSecurity class to provide reasons why the upload security would have changed.

As well as this, a source is now provided from the location which called for the upload's security status to be updated as they are several (e.g. post creator, topic security updater, rake tasks, manual change).
2021-01-29 09:03:44 +10:00
jbrw
2bcca46cc5
FEATURE - ImageMagick jpeg quality (#11004)
* FEATURE - Add SiteSettings to control JPEG image quality

`recompress_original_jpg_quality` - the maximum quality of a newly
uploaded file.

`image_preview_jpg_quality` - the maximum quality of OptimizedImages
2020-10-23 12:38:28 -04:00
Bianca Nenciu
43e52a7dc1
DEV: Remove gifsicle dependency (#10357)
Dependency on gifsicle, allow_animated_avatars and allow_animated_thumbnails
site settings were all removed. Animated GIF images are still allowed, but
the generated optimized images are no longer animated for those (which were
used for avatars and thumbnails).

The added 'animated' is populated by extracting information using FastImage.
This field was used to selectively reoptimize old animations. This process
happens in the background.
2020-10-16 13:41:27 +03:00
Martin Brennan
39b2fb8649
FIX: Invalid URLs could raise exceptions when calling UrlHelper.rails_route_from_url (#10782)
Upload.secure_media_url? raised an exceptions when the URL was invalid,
which was a issue in some situations where secure media URLs must be
removed.

For example, sending digests used PrettyText.strip_secure_media,
which used Upload.secure_media_url? to replace secure media with
placeholders. If the URL was invalid, then an exception would be raised
and left unhandled.

Now instead in UrlHelper.rails_route_from_url we return nil if there is something wrong with the URL.

Co-authored-by: Bianca Nenciu <nenciu.bianca@gmail.com>
2020-09-30 15:20:00 +10:00
Martin Brennan
80268357e7
DEV: Change upload verified column to be integer (#10643)
Per review https://review.discourse.org/t/dev-add-verified-to-uploads-and-fill-in-s3-inventory-10406/14180

Change the verified column for Upload to a verified_status integer column, to avoid having NULL as a weird implicit status.
2020-09-17 13:35:29 +10:00
Martin Brennan
2352f4bfc7
DEV: Replace SECURE_MEDIA_ROUTE const with other methods (#10545)
This is so if the route changes this const won't be around to bite us, use the Rails route methods instead.
2020-08-28 11:28:11 +10:00
Guo Xiang Tan
daddad7fd6
DEV: Update annotations. 2020-08-21 11:36:53 +08:00
Sam Saffron
38e7b1a049
FIX: when destroying uploads clear card and profile background
There is an fk to user_profile that can make destroying uploads fail
if they happen to be set as user profile.

This ensures we clear this information when destroying uploads.

There are more relationships, but this makes some more progress.
2020-08-18 10:55:16 +10:00
Gerhard Schlager
957e851ffe Revert "FIX: Regularly reset unknown extension of uploads"
This reverts commit cc7b24b88b as it shouldn't be needed anymore for new uploads.
2020-08-03 13:37:32 +02:00
Régis Hanol
48b4ed41f5 FIX: uploading an existing image as a site setting
The previous fix (f43c0a5d85) wasn't working for images that were already uploaded.
The "metadata" (eg. 'for_*' and 'secure' attributes) were not added to existing uploads.

Also used 'Upload.get_from_url' is the admin/site_setting controller to properly retrieve
an upload from its URL.

Fixed the Upload::URL_REGEX to use the \h (hexadecimal) for the SHA

Follow-up-to: f43c0a5d85
2020-07-03 19:16:54 +02:00
Jarek Radosz
f28ea4751b FIX: A variable name typo
Not that this whole method is used much anymore.
2020-06-19 19:29:19 +02:00
Michael Brown
d9a02d1336
Revert "Revert "Merge branch 'master' of https://github.com/discourse/discourse""
This reverts commit 20780a1eee.

* SECURITY: re-adds accidentally reverted commit:
  03d26cd6: ensure embed_url contains valid http(s) uri
* when the merge commit e62a85cf was reverted, git chose the 2660c2e2 parent to land on
  instead of the 03d26cd6 parent (which contains security fixes)
2020-05-23 00:56:13 -04:00
Jeff Atwood
20780a1eee Revert "Merge branch 'master' of https://github.com/discourse/discourse"
This reverts commit e62a85cf6f, reversing
changes made to 2660c2e21d.
2020-05-22 20:25:56 -07:00
Martin Brennan
c0779df99d
FIX: Remove access control post FK from uploads (#9853) 2020-05-22 11:20:25 +10:00
David Taylor
03818e642a
FEATURE: Include optimized thumbnails for topics (#9215)
This introduces new APIs for obtaining optimized thumbnails for topics. There are a few building blocks required for this:

- Introduces new `image_upload_id` columns on the `posts` and `topics` table. This replaces the old `image_url` column, which means that thumbnails are now restricted to uploads. Hotlinked thumbnails are no longer possible. In normal use (with pull_hotlinked_images enabled), this has no noticeable impact

- A migration attempts to match existing urls to upload records. If a match cannot be found then the posts will be queued for rebake

- Optimized thumbnails are generated during post_process_cooked. If thumbnails are missing when serializing a topic list, then a sidekiq job is queued

- Topic lists and topics now include a `thumbnails` key, which includes all the available images:
   ```
   "thumbnails": [
   {
     "max_width": null,
     "max_height": null,
     "url": "//example.com/original-image.png",
     "width": 1380,
     "height": 1840
   },
   {
     "max_width": 1024,
     "max_height": 1024,
     "url": "//example.com/optimized-image.png",
     "width": 768,
     "height": 1024
   }
   ]
  ```

- Themes can request additional thumbnail sizes by using a modifier in their `about.json` file:
   ```
    "modifiers": {
      "topic_thumbnail_sizes": [
        [200, 200],
        [800, 800]
      ],
      ...
  ```
  Remember that these are generated asynchronously, so your theme should include logic to fallback to other available thumbnails if your requested size has not yet been generated

- Two new raw plugin outlets are introduced, to improve the customisability of the topic list. `topic-list-before-columns` and `topic-list-before-link`
2020-05-05 09:07:50 +01:00
Sam Saffron
d0d5a138c3
DEV: stop freezing frozen strings
We have the `# frozen_string_literal: true` comment on all our
files. This means all string literals are frozen. There is no need
to call #freeze on any literals.

For files with `# frozen_string_literal: true`

```
puts %w{a b}[0].frozen?
=> true

puts "hi".frozen?
=> true

puts "a #{1} b".frozen?
=> true

puts ("a " + "b").frozen?
=> false

puts (-("a " + "b")).frozen?
=> true
```

For more details see: https://samsaffron.com/archive/2018/02/16/reducing-string-duplication-in-ruby
2020-04-30 16:48:53 +10:00
Martin Brennan
cd1c7d7560
FIX: Copying image markdown for secure media loading full image (#9488)
* When copying the markdown for an image between posts, we were not adding the srcset and data-small-image attributes which are done by calling optimize_image! in cooked post processor
* Refactored the code which was confusing in its current state (the consider_for_reuse method was super confusing) and fixed the issue
2020-04-24 10:29:02 +10:00
Martin Brennan
0388653a4d
DEV: Upload and secure media retroactive rake task improvements (#9027)
* Add uploads:sync_s3_acls rake task to ensure the ACLs in S3 are the correct (public-read or private) setting based on upload security

* Improved uploads:disable_secure_media to be more efficient and provide better messages to the user.

* Rename uploads:ensure_correct_acl task to uploads:secure_upload_analyse_and_update as it does more than check the ACL

* Many improvements to uploads:secure_upload_analyse_and_update

* Make sure that upload.access_control_post is unscoped so deleted posts are still fetched, because they still affect the security of the upload.

* Add escape hatch for capture_stdout in the form of RAILS_ENABLE_TEST_STDOUT. If provided the capture_stdout code will be ignored, so you can see the output if you need.
2020-03-03 10:03:58 +11:00
Martin Brennan
56b16bc68e
FIX: Never allow custom emoji to be marked secure (#8965)
* Because custom emoji count as post "uploads" we were
marking them as secure when updating the secure status for post uploads.
* We were also giving them an access control post id, which meant
broken image previews from 403 errors in the admin custom emoji list.
* We now check if an upload is used as a custom emoji and do not
assign the access control post + never mark as secure.
2020-02-14 11:17:09 +10:00
Martin Brennan
1150cd4621
FIX: Stop secure media URLs being censored too liberally in emails (#8817)
For example /t/ URLs were being replaced if they contained secure-media-uploads so if you made a topic called "Secure Media Uploads Are Cool" the View Topic link in the user notifications would be stripped out.

Refactored code so this secure URL detection happens in one place.
2020-01-30 16:19:14 +10:00
Martin Brennan
ab3bda6cd0
FIX: Mitigate issue where legacy pre-secure hotlinked media would not be redownloaded (#8802)
Basically, say you had already downloaded a certain image from a certain URL
using pull_hotlinked_images and the onebox. The upload would be stored
by its sha as an upload record. Whenever you linked to the same URL again
in a post (e.g. in our case an og:image on review.discourse) we would
would reuse the original upload record because of the sha1.

However when you turned on secure media this could cause problems as
the first post that uses that upload after secure media is enabled
will set the access control post for the upload to the new post.
Then if the post is deleted every single onebox/link to that same image
URL will fail forever with 403 as the secure-media-uploads URL fails
if the access control post has been deleted.

To fix this when cooking posts and pulling hotlinked images, we only
allow using an original upload by URL if its access control post
matches the current post, and if the original_sha1 is filled in,
meaning it was uploaded AFTER secure media was enabled. otherwise
we just redownload the media again to be safe, as the URL will always
be new then.
2020-01-29 10:11:38 +10:00
Martin Brennan
45b37a8bd1
FIX: Resolve pull hotlinked image and broken link issues for secure media URLs (#8777)
When pull_hotlinked_images tried to run on posts with secure media (which had already been downloaded from external sources) we were getting a 404 when trying to download the image because the secure endpoint doesn't allow anon downloads.

Also, we were getting into an infinite loop of pull_hotlinked_images because the job didn't consider the secure media URLs as "downloaded" already so it kept trying to download them over and over.

In this PR I have also refactored secure-media-upload URL checks and mutations into single source of truth in Upload, adding a SECURE_MEDIA_ROUTE constant to check URLs against too.
2020-01-24 11:59:30 +10:00
Martin Brennan
1b3b0708c0
FEATURE: Update upload security status on post move, topic conversion, category change (#8731)
Add TopicUploadSecurityManager to handle post moves. When a post moves around or a topic changes between categories and public/private message status the uploads connected to posts in the topic need to have their secure status updated, depending on the security context the topic now lives in.
2020-01-23 12:01:10 +10:00
Martin Brennan
7c32411881
FEATURE: Secure media allowing duplicated uploads with category-level privacy and post-based access rules (#8664)
### General Changes and Duplication

* We now consider a post `with_secure_media?` if it is in a read-restricted category.
* When uploading we now set an upload's secure status straight away.
* When uploading if `SiteSetting.secure_media` is enabled, we do not check to see if the upload already exists using the `sha1` digest of the upload. The `sha1` column of the upload is filled with a `SecureRandom.hex(20)` value which is the same length as `Upload::SHA1_LENGTH`. The `original_sha1` column is filled with the _real_ sha1 digest of the file. 
* Whether an upload `should_be_secure?` is now determined by whether the `access_control_post` is `with_secure_media?` (if there is no access control post then we leave the secure status as is).
* When serializing the upload, we now cook the URL if the upload is secure. This is so it shows up correctly in the composer preview, because we set secure status on upload.

### Viewing Secure Media

* The secure-media-upload URL will take the post that the upload is attached to into account via `Guardian.can_see?` for access permissions
* If there is no `access_control_post` then we just deliver the media. This should be a rare occurrance and shouldn't cause issues as the `access_control_post` is set when `link_post_uploads` is called via `CookedPostProcessor`

### Removed

We no longer do any of these because we do not reuse uploads by sha1 if secure media is enabled.

* We no longer have a way to prevent cross-posting of a secure upload from a private context to a public context.
* We no longer have to set `secure: false` for uploads when uploading for a theme component.
2020-01-16 13:50:27 +10:00