From 763dffa32823ff15603f40ed8dc467d3e3a0369b Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 4 Jun 2023 04:16:48 +0200 Subject: [PATCH] Always strip exif data and drop user setting for it Some imagemagick-versions (I tested Ubuntu 22.04 and debian bullseye) always loose exif data when converting from jpg to webp. So this made our CI fail now, but even if it wasn't failing before, some pods always had and have versions which might loose the information anyway. So having a setting to keep exif information is kinda pointless, if we can't guarantee that the information isn't lost. Also, diaspora isn't a photo sharing platform and we don't display exif information anywhere, so I think we should just always strip exif data (which was already the default before), as we don't need them. --- app/controllers/users_controller.rb | 12 ------------ app/models/photo.rb | 2 -- app/models/user.rb | 3 +-- app/serializers/export/user_serializer.rb | 1 - app/uploaders/unprocessed_image.rb | 8 +------- app/views/users/_privacy_settings.haml | 16 ---------------- config/locales/diaspora/en.yml | 1 - ...0230604015559_remove_strip_exif_from_users.rb | 7 +++++++ lib/archive_importer.rb | 2 +- lib/schemas/archive-format.json | 1 - spec/integration/exporter_spec.rb | 2 +- spec/lib/archive_importer_spec.rb | 3 --- spec/models/photo_spec.rb | 8 +------- spec/serializers/export/user_serializer_spec.rb | 1 - spec/support/account_matchers.rb | 1 - spec/support/fixture_builder.rb | 2 +- 16 files changed, 13 insertions(+), 57 deletions(-) create mode 100644 db/migrate/20230604015559_remove_strip_exif_from_users.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 7e7e10b81..2fc3cbec8 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -31,18 +31,6 @@ class UsersController < ApplicationController render :edit end - def update_privacy_settings - privacy_params = params.fetch(:user).permit(:strip_exif) - - if current_user.update(strip_exif: privacy_params[:strip_exif]) - flash[:notice] = t("users.update.settings_updated") - else - flash[:error] = t("users.update.settings_not_updated") - end - - redirect_back fallback_location: privacy_settings_path - end - def destroy if params[:user] && params[:user][:current_password] && current_user.valid_password?(params[:user][:current_password]) current_user.close_account! diff --git a/app/models/photo.rb b/app/models/photo.rb index e9fbe5d20..b199583b7 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -76,8 +76,6 @@ class Photo < ApplicationRecord photo = new(params.to_hash.stringify_keys.slice(*column_names, "author")) photo.random_string = SecureRandom.hex(10) - photo.unprocessed_image.strip_exif = photo.author.owner.strip_exif - if params[:user_file] image_file = params.delete(:user_file) photo.unprocessed_image.store! image_file diff --git a/app/models/user.rb b/app/models/user.rb index 35256876b..7b8a9d01c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -569,7 +569,6 @@ class User < ApplicationRecord self.remove_export = true self.remove_exported_photos_file = true self[:disable_mail] = true - self[:strip_exif] = true self[:email] = "deletedaccount_#{self[:id]}@example.org" random_password = SecureRandom.hex(20) @@ -612,7 +611,7 @@ class User < ApplicationRecord attributes.keys - %w(id username encrypted_password created_at updated_at locked_at serialized_private_key getting_started disable_mail show_community_spotlight_in_stream - strip_exif email remove_after export exporting + email remove_after export exporting exported_photos_file exporting_photos) end end diff --git a/app/serializers/export/user_serializer.rb b/app/serializers/export/user_serializer.rb index dcf435b6b..8d43312fa 100644 --- a/app/serializers/export/user_serializer.rb +++ b/app/serializers/export/user_serializer.rb @@ -10,7 +10,6 @@ module Export :show_community_spotlight_in_stream, :auto_follow_back, :auto_follow_back_aspect, - :strip_exif, :blocks has_one :profile, serializer: FederationEntitySerializer has_many :contact_groups, each_serializer: Export::AspectSerializer diff --git a/app/uploaders/unprocessed_image.rb b/app/uploaders/unprocessed_image.rb index a653aa34f..7d04a4e77 100644 --- a/app/uploaders/unprocessed_image.rb +++ b/app/uploaders/unprocessed_image.rb @@ -7,12 +7,6 @@ class UnprocessedImage < CarrierWave::Uploader::Base include CarrierWave::MiniMagick - attr_accessor :strip_exif - - def strip_exif - @strip_exif || false - end - def store_dir "uploads/images" end @@ -40,7 +34,7 @@ class UnprocessedImage < CarrierWave::Uploader::Base manipulate! do |img| img.combine_options do |i| i.auto_orient - i.strip if strip_exif + i.strip end img = yield(img) if block_given? diff --git a/app/views/users/_privacy_settings.haml b/app/views/users/_privacy_settings.haml index 9a881bc51..caecb7cfe 100644 --- a/app/views/users/_privacy_settings.haml +++ b/app/views/users/_privacy_settings.haml @@ -1,19 +1,3 @@ -.row - .col-md-12 - %h3 - = t(".title") - - = form_for current_user, url: update_privacy_settings_path, html: {method: :put} do |f| - = f.error_messages - - = f.fields_for :stream_preferences do - .checkbox#stream_prefs - = f.label :strip_exif do - = f.check_box :strip_exif - = t(".strip_exif") - = f.submit t("users.edit.change"), class: "btn btn-primary pull-right" -%hr - .row .col-md-12 %h3 diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index e47713b4f..fea1bf946 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -1331,7 +1331,6 @@ en: privacy_settings: title: "Privacy settings" - strip_exif: "Strip metadata such as location, author, and camera model from uploaded images (recommended)" ignored_users: "Ignored users" stop_ignoring: "Stop ignoring" no_user_ignored_message: "You are not currently ignoring any other user" diff --git a/db/migrate/20230604015559_remove_strip_exif_from_users.rb b/db/migrate/20230604015559_remove_strip_exif_from_users.rb new file mode 100644 index 000000000..5647d4add --- /dev/null +++ b/db/migrate/20230604015559_remove_strip_exif_from_users.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class RemoveStripExifFromUsers < ActiveRecord::Migration[6.1] + def change + remove_column :users, :strip_exif, :boolean, default: true + end +end diff --git a/lib/archive_importer.rb b/lib/archive_importer.rb index 8776edca8..64cd0dc58 100644 --- a/lib/archive_importer.rb +++ b/lib/archive_importer.rb @@ -123,7 +123,7 @@ class ArchiveImporter end def import_settings - allowed_keys = %w[language show_community_spotlight_in_stream strip_exif] + allowed_keys = %w[language show_community_spotlight_in_stream] convert_keys(archive_hash["user"], allowed_keys).each do |key, value| user.update(key => value) unless value.nil? end diff --git a/lib/schemas/archive-format.json b/lib/schemas/archive-format.json index 79105232d..f2a3e6ed9 100644 --- a/lib/schemas/archive-format.json +++ b/lib/schemas/archive-format.json @@ -19,7 +19,6 @@ "null" ] }, - "strip_exif": { "type": "boolean" }, "blocks": { "type": "array", "items" : { diff --git a/spec/integration/exporter_spec.rb b/spec/integration/exporter_spec.rb index e1134d3c0..cfe197b6a 100644 --- a/spec/integration/exporter_spec.rb +++ b/spec/integration/exporter_spec.rb @@ -24,7 +24,7 @@ describe Diaspora::Exporter do user_properties = build_property_hash( user, %i[email username language disable_mail show_community_spotlight_in_stream auto_follow_back - auto_follow_back_aspect strip_exif], + auto_follow_back_aspect], private_key: :serialized_private_key ) diff --git a/spec/lib/archive_importer_spec.rb b/spec/lib/archive_importer_spec.rb index 2ad211965..7c0693dd5 100644 --- a/spec/lib/archive_importer_spec.rb +++ b/spec/lib/archive_importer_spec.rb @@ -116,7 +116,6 @@ describe ArchiveImporter do "contact_groups" => [{ "name" => "Follow" }], - "strip_exif" => false, "show_community_spotlight_in_stream" => false, "language" => "ru", "auto_follow_back" => true, @@ -130,7 +129,6 @@ describe ArchiveImporter do archive_importer.import }.not_to raise_error - expect(archive_importer.user.strip_exif).to eq(false) expect(archive_importer.user.show_community_spotlight_in_stream).to eq(false) expect(archive_importer.user.language).to eq("ru") expect(archive_importer.user.auto_follow_back).to eq(true) @@ -142,7 +140,6 @@ describe ArchiveImporter do archive_importer.import(import_settings: false) }.not_to raise_error - expect(archive_importer.user.strip_exif).to eq(true) expect(archive_importer.user.show_community_spotlight_in_stream).to eq(true) expect(archive_importer.user.language).to eq("en") expect(archive_importer.user.auto_follow_back).to eq(false) diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index 34c24b936..89e6646a2 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -147,13 +147,7 @@ describe Photo, :type => :model do FileUtils.rm_r Dir.glob(File.join(public_path, "uploads/images/*")) end - it "should preserve EXIF data in according to user preference" do - image = image_from a_photo_sent_by(alice) - - expect(image.exif.length).not_to eq(0) - end - - it "should not preserve EXIF in according to user preference" do + it "should strip EXIF data" do image = image_from a_photo_sent_by(bob) expect(image.exif.length).to eq(0) diff --git a/spec/serializers/export/user_serializer_spec.rb b/spec/serializers/export/user_serializer_spec.rb index 9720b5459..bab2f1afe 100644 --- a/spec/serializers/export/user_serializer_spec.rb +++ b/spec/serializers/export/user_serializer_spec.rb @@ -14,7 +14,6 @@ describe Export::UserSerializer do show_community_spotlight_in_stream: user.show_community_spotlight_in_stream, auto_follow_back: user.auto_follow_back, auto_follow_back_aspect: user.auto_follow_back_aspect, - strip_exif: user.strip_exif, blocks: user.blocks ) end diff --git a/spec/support/account_matchers.rb b/spec/support/account_matchers.rb index 1ccf5542c..b22f36f05 100644 --- a/spec/support/account_matchers.rb +++ b/spec/support/account_matchers.rb @@ -34,7 +34,6 @@ RSpec::Matchers.define :be_a_clear_account do ].map {|attribute| user[attribute] } user.disable_mail && - user.strip_exif && !user.getting_started && !user.show_community_spotlight_in_stream && !user.post_default_public && diff --git a/spec/support/fixture_builder.rb b/spec/support/fixture_builder.rb index 33082869b..25a4fb179 100644 --- a/spec/support/fixture_builder.rb +++ b/spec/support/fixture_builder.rb @@ -2,7 +2,7 @@ def create_basic_users # Users - alice = FactoryBot.create(:user_with_aspect, username: "alice", strip_exif: false) + alice = FactoryBot.create(:user_with_aspect, username: "alice") alices_aspect = alice.aspects.where(name: "generic").first eve = FactoryBot.create(:user_with_aspect, username: "eve")