From ba826ff6fa264682bea5e315fc121e0200c5a20b Mon Sep 17 00:00:00 2001 From: nonamethanks Date: Fri, 26 Feb 2021 19:45:01 +0100 Subject: [PATCH 01/27] Sources: get correct mastodon page url --- app/logical/sources/strategies/mastodon.rb | 4 ++-- test/unit/sources/mastodon_test.rb | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/logical/sources/strategies/mastodon.rb b/app/logical/sources/strategies/mastodon.rb index 3fef4810e..5d353fd7a 100644 --- a/app/logical/sources/strategies/mastodon.rb +++ b/app/logical/sources/strategies/mastodon.rb @@ -85,7 +85,7 @@ module Sources::Strategies end def artist_name_from_url - url[NAMED_PROFILE, :artist_name] + urls.map { |url| url[NAMED_PROFILE, :artist_name] }.compact.first end def other_names @@ -93,7 +93,7 @@ module Sources::Strategies end def account_id - url[ID_PROFILE, :account_id] || api_response.account_id + urls.map { |url| url[ID_PROFILE, :account_id] }.compact.first || api_response.account_id end def status_id_from_url diff --git a/test/unit/sources/mastodon_test.rb b/test/unit/sources/mastodon_test.rb index 59d31d0f7..95a3f26ad 100644 --- a/test/unit/sources/mastodon_test.rb +++ b/test/unit/sources/mastodon_test.rb @@ -95,6 +95,10 @@ module Sources should "fetch the source data" do assert_equal("evazion", @site.artist_name) end + + should "correctly get the page url" do + assert_equal(@ref, @site.page_url) + end end context "A baraag url" do From e75489aa94a4ee9d540d81de10865914495b8905 Mon Sep 17 00:00:00 2001 From: nonamethanks Date: Tue, 2 Mar 2021 17:05:47 +0100 Subject: [PATCH 02/27] Twitter: update common hashtag regexes --- app/logical/sources/strategies/twitter.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/logical/sources/strategies/twitter.rb b/app/logical/sources/strategies/twitter.rb index 593a049f6..55c09de30 100644 --- a/app/logical/sources/strategies/twitter.rb +++ b/app/logical/sources/strategies/twitter.rb @@ -30,6 +30,7 @@ module Sources::Strategies /(? Date: Thu, 4 Mar 2021 20:47:43 +0100 Subject: [PATCH 03/27] Change "Inviter" to "Promoter" It's kind of old and misleading --- app/views/users/_statistics.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/users/_statistics.html.erb b/app/views/users/_statistics.html.erb index f72744ad8..34799f6fd 100644 --- a/app/views/users/_statistics.html.erb +++ b/app/views/users/_statistics.html.erb @@ -63,7 +63,7 @@ <% end %> - Inviter + Promoter <% if user.inviter %> <%= link_to_user user.inviter %> <%= link_to "»", users_path(search: { inviter: { name: user.inviter.name }}) %> <% else %> From 7ed674f6825001f85d0aa2ab70454732b6a65d8f Mon Sep 17 00:00:00 2001 From: smow <7839556+smowtenshi@users.noreply.github.com> Date: Thu, 4 Mar 2021 22:07:52 +0100 Subject: [PATCH 04/27] Section removed as per comments --- app/views/users/_statistics.html.erb | 9 --------- 1 file changed, 9 deletions(-) diff --git a/app/views/users/_statistics.html.erb b/app/views/users/_statistics.html.erb index 34799f6fd..29b06833b 100644 --- a/app/views/users/_statistics.html.erb +++ b/app/views/users/_statistics.html.erb @@ -62,15 +62,6 @@ <% end %> - - Promoter - <% if user.inviter %> - <%= link_to_user user.inviter %> <%= link_to "»", users_path(search: { inviter: { name: user.inviter.name }}) %> - <% else %> - None - <% end %> - - Level From 4492610dfe11b7041f0995d856c595fdeb74a360 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 4 Mar 2021 20:07:19 -0600 Subject: [PATCH 05/27] rate limits: rework rate limit implementation. Rework the rate limit implementation to make it more flexible: * Allow setting different rate limits for different actions. Before we had a single rate limit for all write actions. Now different controller endpoints can have different limits. * Allow actions to be rate limited by user ID, by IP address, or both. Before actions were only limited by user ID, which meant non-logged-in actions like creating new accounts or attempting to login couldn't be rate limited. Also, because actions were limited by user ID only, you could use multiple accounts with the same IP to get around limits. Other changes: * Remove the API Limit field from user profile pages. * Remove the `remaining_api_limit` field from the `/profile.json` endpoint. * Rename the `X-Api-Limit` header to `X-Rate-Limit` and change it from a number to a JSON object containing all the rate limit info (including the refill rate, the burst factor, the cost of the call, and the current limits). * Fix a potential race condition where, if you flooded requests fast enough, you could exceed the rate limit. This was because we checked and updated the rate limit in two separate steps, which was racy; simultaneous requests could pass the check before the update happened. The new code uses some tricky SQL to check and update multiple limits in a single statement. --- app/controllers/application_controller.rb | 23 +++--- app/logical/danbooru_maintenance.rb | 2 +- app/logical/rate_limiter.rb | 30 +++++++ app/models/rate_limit.rb | 53 +++++++++++++ app/models/token_bucket.rb | 41 ---------- app/models/user.rb | 5 -- app/policies/user_policy.rb | 2 +- app/views/users/_statistics.html.erb | 8 -- .../20170106012138_create_token_buckets.rb | 2 +- ..._replace_token_buckets_with_rate_limits.rb | 24 ++++++ db/structure.sql | 78 ++++++++++++++----- test/factories/rate_limit.rb | 8 ++ .../functional/application_controller_test.rb | 2 +- test/unit/rate_limit_test.rb | 72 +++++++++++++++++ test/unit/token_bucket_test.rb | 45 ----------- 15 files changed, 260 insertions(+), 135 deletions(-) create mode 100644 app/logical/rate_limiter.rb create mode 100644 app/models/rate_limit.rb delete mode 100644 app/models/token_bucket.rb create mode 100644 db/migrate/20210303195217_replace_token_buckets_with_rate_limits.rb create mode 100644 test/factories/rate_limit.rb create mode 100644 test/unit/rate_limit_test.rb delete mode 100644 test/unit/token_bucket_test.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5b36a3175..8c8237bdd 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,8 +2,6 @@ class ApplicationController < ActionController::Base include Pundit helper_method :search_params - class ApiLimitError < StandardError; end - self.responder = ApplicationResponder skip_forgery_protection if: -> { SessionLoader.new(request).has_api_authentication? } @@ -74,17 +72,16 @@ class ApplicationController < ActionController::Base def api_check return if CurrentUser.is_anonymous? || request.get? || request.head? - if CurrentUser.user.token_bucket.nil? - TokenBucket.create_default(CurrentUser.user) - CurrentUser.user.reload - end + rate_limiter = RateLimiter.new( + "write", + [CurrentUser.user.cache_key], + cost: 1, + rate: CurrentUser.user.api_regen_multiplier, + burst: CurrentUser.user.api_burst_limit + ) - throttled = CurrentUser.user.token_bucket.throttled? - headers["X-Api-Limit"] = CurrentUser.user.token_bucket.token_count.to_s - - if throttled - raise ApiLimitError, "too many requests" - end + headers["X-Rate-Limit"] = rate_limiter.to_json + rate_limiter.limit! end def rescue_exception(exception) @@ -113,7 +110,7 @@ class ApplicationController < ActionController::Base render_error_page(410, exception, template: "static/pagination_error", message: "You cannot go beyond page #{CurrentUser.user.page_limit}.") when Post::SearchError render_error_page(422, exception, template: "static/tag_limit_error", message: "You cannot search for more than #{CurrentUser.tag_query_limit} tags at a time.") - when ApiLimitError + when RateLimiter::RateLimitError render_error_page(429, exception) when NotImplementedError render_error_page(501, exception, message: "This feature isn't available: #{exception.message}") diff --git a/app/logical/danbooru_maintenance.rb b/app/logical/danbooru_maintenance.rb index c990dcfae..48acb7c0e 100644 --- a/app/logical/danbooru_maintenance.rb +++ b/app/logical/danbooru_maintenance.rb @@ -5,13 +5,13 @@ module DanbooruMaintenance safely { Upload.prune! } safely { PostPruner.prune! } safely { PostAppealForumUpdater.update_forum! } + safely { RateLimit.prune! } safely { regenerate_post_counts! } end def daily safely { Delayed::Job.where('created_at < ?', 45.days.ago).delete_all } safely { PostDisapproval.prune! } - safely { TokenBucket.prune! } safely { BulkUpdateRequestPruner.warn_old } safely { BulkUpdateRequestPruner.reject_expired } safely { Ban.prune! } diff --git a/app/logical/rate_limiter.rb b/app/logical/rate_limiter.rb new file mode 100644 index 000000000..e8fc52430 --- /dev/null +++ b/app/logical/rate_limiter.rb @@ -0,0 +1,30 @@ +class RateLimiter + class RateLimitError < StandardError; end + + attr_reader :action, :keys, :cost, :rate, :burst + + def initialize(action, keys = ["*"], cost: 1, rate: 1, burst: 1) + @action = action + @keys = keys + @cost = cost + @rate = rate + @burst = burst + end + + def limit! + raise RateLimitError if limited? + end + + def limited? + rate_limits.any?(&:limited?) + end + + def as_json(options = {}) + hash = rate_limits.map { |limit| [limit.key, limit.points] }.to_h + super(options).except("keys", "rate_limits").merge(limits: hash) + end + + def rate_limits + @rate_limits ||= RateLimit.create_or_update!(action: action, keys: keys, cost: cost, rate: rate, burst: burst) + end +end diff --git a/app/models/rate_limit.rb b/app/models/rate_limit.rb new file mode 100644 index 000000000..3236f1ac4 --- /dev/null +++ b/app/models/rate_limit.rb @@ -0,0 +1,53 @@ +class RateLimit < ApplicationRecord + scope :expired, -> { where("updated_at < ?", 1.hour.ago) } + + def self.prune! + expired.delete_all + end + + # `action` is the action being limited. Usually a controller endpoint. + # `keys` is who is being limited. Usually a [user, ip] pair, meaning the action is limited both by the user's ID and their IP. + # `cost` is the number of points the action costs. + # `rate` is the number of points per second that are refilled. + # `burst` is the maximum number of points that can be saved up. + def self.create_or_update!(action:, keys:, cost:, rate:, burst:) + # { key0: keys[0], ..., keyN: keys[N] } + key_params = keys.map.with_index { |key, i| [:"key#{i}", key] }.to_h + + # (created_at, updated_at, action, keyN, points) + values = keys.map.with_index { |key, i| "(:now, :now, :action, :key#{i}, :points)" } + + # Do an upsert, creating a new rate limit object for each key that doesn't + # already exist, and updating the limit for each limit that already exists. + # + # https://www.postgresql.org/docs/current/sql-insert.html#SQL-ON-CONFLICT + sql = <<~SQL + INSERT INTO rate_limits (created_at, updated_at, action, key, points) + VALUES #{values.join(", ")} + ON CONFLICT (action, key) DO UPDATE SET + updated_at = :now, + limited = rate_limits.points + :rate * EXTRACT(epoch FROM (:now - rate_limits.updated_at)) < 0, + points = + CASE + WHEN rate_limits.points + :rate * EXTRACT(epoch FROM (:now - rate_limits.updated_at)) < 0 THEN + LEAST(:burst, rate_limits.points + :rate * EXTRACT(epoch FROM (:now - rate_limits.updated_at))) + ELSE + LEAST(:burst, rate_limits.points + :rate * EXTRACT(epoch FROM (:now - rate_limits.updated_at))) - :cost + END + RETURNING * + SQL + + sql_params = { + now: Time.zone.now, + action: action, + rate: rate, + burst: burst, + cost: cost, + points: burst - cost, + **key_params + } + + rate_limits = RateLimit.find_by_sql([sql, sql_params]) + rate_limits + end +end diff --git a/app/models/token_bucket.rb b/app/models/token_bucket.rb deleted file mode 100644 index 96ddf74ab..000000000 --- a/app/models/token_bucket.rb +++ /dev/null @@ -1,41 +0,0 @@ -class TokenBucket < ApplicationRecord - self.primary_key = "user_id" - belongs_to :user - - def self.prune! - where("last_touched_at < ?", 1.day.ago).delete_all - end - - def self.create_default(user) - TokenBucket.create(user_id: user.id, token_count: user.api_burst_limit, last_touched_at: Time.now) - end - - def accept? - token_count >= 1 - end - - def add! - now = Time.now - TokenBucket.where(user_id: user_id).update_all(["token_count = least(token_count + (? * extract(epoch from ? - last_touched_at)), ?), last_touched_at = ?", user.api_regen_multiplier, now, user.api_burst_limit, now]) - - # estimate the token count to avoid reloading - self.token_count += user.api_regen_multiplier * (now - last_touched_at) - self.token_count = user.api_burst_limit if token_count > user.api_burst_limit - end - - def consume! - TokenBucket.where(user_id: user_id).update_all("token_count = greatest(0, token_count - 1)") - self.token_count -= 1 - end - - def throttled? - add! - - if accept? - consume! - return false - else - return true - end - end -end diff --git a/app/models/user.rb b/app/models/user.rb index d8ad0b507..22dfa82b1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -134,7 +134,6 @@ class User < ApplicationRecord has_many :user_events, dependent: :destroy has_one :recent_ban, -> {order("bans.id desc")}, :class_name => "Ban" - has_one :token_bucket has_one :email_address, dependent: :destroy has_many :api_keys, dependent: :destroy has_many :note_versions, :foreign_key => "updater_id" @@ -539,10 +538,6 @@ class User < ApplicationRecord User.api_burst_limit(level) end - def remaining_api_limit - token_bucket.try(:token_count) || api_burst_limit - end - def statement_timeout User.statement_timeout(level) end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index faf36bec7..b5bef3ba9 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -62,7 +62,7 @@ class UserPolicy < ApplicationPolicy comment_threshold default_image_size favorite_tags blacklisted_tags time_zone per_page custom_style favorite_count api_regen_multiplier - api_burst_limit remaining_api_limit statement_timeout + api_burst_limit statement_timeout favorite_group_limit favorite_limit tag_query_limit max_saved_searches theme ] diff --git a/app/views/users/_statistics.html.erb b/app/views/users/_statistics.html.erb index f72744ad8..cc7625f80 100644 --- a/app/views/users/_statistics.html.erb +++ b/app/views/users/_statistics.html.erb @@ -265,14 +265,6 @@ (<%= link_to_wiki "help", "help:api" %>) - - - API Limits - - <%= CurrentUser.user.remaining_api_limit %> - / <%= CurrentUser.user.api_burst_limit %> (may not be up to date) - - <% end %> diff --git a/db/migrate/20170106012138_create_token_buckets.rb b/db/migrate/20170106012138_create_token_buckets.rb index a41a7067a..0b442ae6f 100644 --- a/db/migrate/20170106012138_create_token_buckets.rb +++ b/db/migrate/20170106012138_create_token_buckets.rb @@ -5,6 +5,6 @@ class CreateTokenBuckets < ActiveRecord::Migration[4.2] end def down - raise NotImplementedError + drop_table :token_buckets end end diff --git a/db/migrate/20210303195217_replace_token_buckets_with_rate_limits.rb b/db/migrate/20210303195217_replace_token_buckets_with_rate_limits.rb new file mode 100644 index 000000000..7002d5a75 --- /dev/null +++ b/db/migrate/20210303195217_replace_token_buckets_with_rate_limits.rb @@ -0,0 +1,24 @@ +require_relative "20170106012138_create_token_buckets" + +class ReplaceTokenBucketsWithRateLimits < ActiveRecord::Migration[6.1] + def change + revert CreateTokenBuckets + + create_table :rate_limits do |t| + t.timestamps null: false + t.boolean :limited, null: false, default: false + t.float :points, null: false + t.string :action, null: false + t.string :key, null: false + + t.index [:key, :action], unique: true + end + + reversible do |dir| + dir.up do + execute "ALTER TABLE rate_limits SET UNLOGGED" + execute "ALTER TABLE rate_limits SET (fillfactor = 50)" + end + end + end +end diff --git a/db/structure.sql b/db/structure.sql index e7549407b..13c02f9c8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2929,6 +2929,41 @@ CREATE SEQUENCE public.posts_id_seq ALTER SEQUENCE public.posts_id_seq OWNED BY public.posts.id; +-- +-- Name: rate_limits; Type: TABLE; Schema: public; Owner: - +-- + +CREATE UNLOGGED TABLE public.rate_limits ( + id bigint NOT NULL, + created_at timestamp(6) without time zone NOT NULL, + updated_at timestamp(6) without time zone NOT NULL, + limited boolean DEFAULT false NOT NULL, + points double precision NOT NULL, + action character varying NOT NULL, + key character varying NOT NULL +) +WITH (fillfactor='50'); + + +-- +-- Name: rate_limits_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.rate_limits_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: rate_limits_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.rate_limits_id_seq OWNED BY public.rate_limits.id; + + -- -- Name: saved_searches; Type: TABLE; Schema: public; Owner: - -- @@ -3085,17 +3120,6 @@ CREATE SEQUENCE public.tags_id_seq ALTER SEQUENCE public.tags_id_seq OWNED BY public.tags.id; --- --- Name: token_buckets; Type: TABLE; Schema: public; Owner: - --- - -CREATE UNLOGGED TABLE public.token_buckets ( - user_id integer, - last_touched_at timestamp without time zone NOT NULL, - token_count real NOT NULL -); - - -- -- Name: uploads; Type: TABLE; Schema: public; Owner: - -- @@ -4352,6 +4376,13 @@ ALTER TABLE ONLY public.post_votes ALTER COLUMN id SET DEFAULT nextval('public.p ALTER TABLE ONLY public.posts ALTER COLUMN id SET DEFAULT nextval('public.posts_id_seq'::regclass); +-- +-- Name: rate_limits id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.rate_limits ALTER COLUMN id SET DEFAULT nextval('public.rate_limits_id_seq'::regclass); + + -- -- Name: saved_searches id; Type: DEFAULT; Schema: public; Owner: - -- @@ -4739,6 +4770,14 @@ ALTER TABLE ONLY public.posts ADD CONSTRAINT posts_pkey PRIMARY KEY (id); +-- +-- Name: rate_limits rate_limits_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.rate_limits + ADD CONSTRAINT rate_limits_pkey PRIMARY KEY (id); + + -- -- Name: saved_searches saved_searches_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -7280,6 +7319,13 @@ CREATE INDEX index_posts_on_uploader_id ON public.posts USING btree (uploader_id CREATE INDEX index_posts_on_uploader_ip_addr ON public.posts USING btree (uploader_ip_addr); +-- +-- Name: index_rate_limits_on_key_and_action; Type: INDEX; Schema: public; Owner: - +-- + +CREATE UNIQUE INDEX index_rate_limits_on_key_and_action ON public.rate_limits USING btree (key, action); + + -- -- Name: index_saved_searches_on_labels; Type: INDEX; Schema: public; Owner: - -- @@ -7385,13 +7431,6 @@ CREATE INDEX index_tags_on_name_trgm ON public.tags USING gin (name public.gin_t CREATE INDEX index_tags_on_post_count ON public.tags USING btree (post_count); --- --- Name: index_token_buckets_on_user_id; Type: INDEX; Schema: public; Owner: - --- - -CREATE UNIQUE INDEX index_token_buckets_on_user_id ON public.token_buckets USING btree (user_id); - - -- -- Name: index_uploads_on_referer_url; Type: INDEX; Schema: public; Owner: - -- @@ -7964,6 +8003,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20210127000201'), ('20210127012303'), ('20210214095121'), -('20210214101614'); +('20210214101614'), +('20210303195217'); diff --git a/test/factories/rate_limit.rb b/test/factories/rate_limit.rb new file mode 100644 index 000000000..cfb37ff3f --- /dev/null +++ b/test/factories/rate_limit.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + factory(:rate_limit) do + limited { false } + points { 0 } + action { "test" } + key { "1234" } + end +end diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb index bd96c4a24..0783431f6 100644 --- a/test/functional/application_controller_test.rb +++ b/test/functional/application_controller_test.rb @@ -227,7 +227,7 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest should "fail with a 429 error" do user = create(:user) post = create(:post, rating: "s") - TokenBucket.any_instance.stubs(:throttled?).returns(true) + RateLimit.any_instance.stubs(:limited?).returns(true) put_auth post_path(post), user, params: { post: { rating: "e" } } diff --git a/test/unit/rate_limit_test.rb b/test/unit/rate_limit_test.rb new file mode 100644 index 000000000..d120a494a --- /dev/null +++ b/test/unit/rate_limit_test.rb @@ -0,0 +1,72 @@ +require 'test_helper' + +class RateLimitTest < ActiveSupport::TestCase + context "RateLimit: " do + context "#limit! method" do + should "create a new rate limit object if none exists, or update it if it already exists" do + assert_difference("RateLimit.count", 1) do + RateLimiter.new("write", ["users/1"]).limited? + end + + assert_difference("RateLimit.count", 0) do + RateLimiter.new("write", ["users/1"]).limited? + end + + assert_difference("RateLimit.count", 1) do + RateLimiter.new("write", ["users/1", "ip/1.2.3.4"]).limited? + end + + assert_difference("RateLimit.count", 0) do + RateLimiter.new("write", ["users/1", "ip/1.2.3.4"]).limited? + end + end + + should "include the cost of the first action when initializing the limit" do + limiter = RateLimiter.new("write", ["users/1"], burst: 10, cost: 1) + assert_equal(9, limiter.rate_limits.first.points) + end + + should "be limited if the point count is negative" do + freeze_time + create(:rate_limit, action: "write", key: "users/1", points: -1) + limiter = RateLimiter.new("write", ["users/1"], cost: 1) + + assert_equal(true, limiter.limited?) + assert_equal(-1, limiter.rate_limits.first.points) + end + + should "not be limited if the point count was positive before the action" do + freeze_time + create(:rate_limit, action: "write", key: "users/1", points: 0.01) + limiter = RateLimiter.new("write", ["users/1"], cost: 1) + + assert_equal(false, limiter.limited?) + assert_equal(-0.99, limiter.rate_limits.first.points) + end + + should "refill the points at the correct rate" do + freeze_time + create(:rate_limit, action: "write", key: "users/1", points: -2) + + limiter = RateLimiter.new("write", ["users/1"], cost: 1, rate: 1, burst: 10) + assert_equal(true, limiter.limited?) + assert_equal(-2, limiter.rate_limits.first.points) + + travel 1.second + limiter = RateLimiter.new("write", ["users/1"], cost: 1, rate: 1, burst: 10) + assert_equal(true, limiter.limited?) + assert_equal(-1, limiter.rate_limits.first.points) + + travel 5.second + limiter = RateLimiter.new("write", ["users/1"], cost: 1, rate: 1, burst: 10) + assert_equal(false, limiter.limited?) + assert_equal(3, limiter.rate_limits.first.points) + + travel 60.second + limiter = RateLimiter.new("write", ["users/1"], cost: 1, rate: 1, burst: 10) + assert_equal(false, limiter.limited?) + assert_equal(9, limiter.rate_limits.first.points) + end + end + end +end diff --git a/test/unit/token_bucket_test.rb b/test/unit/token_bucket_test.rb deleted file mode 100644 index 3c96b371a..000000000 --- a/test/unit/token_bucket_test.rb +++ /dev/null @@ -1,45 +0,0 @@ -require 'test_helper' - -class TokenBucketTest < ActiveSupport::TestCase - context "#add!" do - setup do - @user = FactoryBot.create(:user) - TokenBucket.create(user_id: @user.id, last_touched_at: 1.minute.ago, token_count: 0) - end - - should "work" do - @user.token_bucket.add! - assert_operator(@user.token_bucket.token_count, :>, 0) - @user.reload - assert_operator(@user.token_bucket.token_count, :>, 0) - end - end - - context "#consume!" do - setup do - @user = FactoryBot.create(:user) - TokenBucket.create(user_id: @user.id, last_touched_at: 1.minute.ago, token_count: 1) - end - - should "work" do - @user.token_bucket.consume! - assert_operator(@user.token_bucket.token_count, :<, 1) - @user.reload - assert_operator(@user.token_bucket.token_count, :<, 1) - end - end - - context "#throttled?" do - setup do - @user = FactoryBot.create(:user) - TokenBucket.create(user_id: @user.id, last_touched_at: 1.minute.ago, token_count: 0) - end - - should "work" do - assert(!@user.token_bucket.throttled?) - assert_operator(@user.token_bucket.token_count, :<, 60) - @user.reload - assert_operator(@user.token_bucket.token_count, :<, 60) - end - end -end From 413cd34c457ed413b51013fb366a0948b0b4db22 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 5 Mar 2021 04:50:37 -0600 Subject: [PATCH 06/27] rate limits: adjust limits for various actions. * Tie rate limits to both the user's ID and their IP address. * Make each endpoint have separate rate limits. This means that, for example, your post edit rate limit is separate from your post vote rate limit. Before all write actions had a shared rate limit. * Make all write endpoints have rate limits. Before some endpoints, such as voting, favoriting, commenting, or forum posting, weren't subject to rate limits. * Add stricter rate limits for some endpoints: ** 1 per 5 minutes for creating new accounts. ** 1 per minute for login attempts, changing your email address, or for creating mod reports. ** 1 per minute for sending dmails, creating comments, creating forum posts, or creating forum topics. ** 1 per second for voting, favoriting, or disapproving posts. ** These rate limits all have burst factors high enough that they shouldn't affect normal, non-automated users. * Raise the default write rate limit for Gold users from 2 per second to 4 per second, for all other actions not listed above. * Raise the default burst factor to 200 for all other actions not listed above. Before it was 10 for Members, 30 for Gold, and 60 for Platinum. --- app/controllers/application_controller.rb | 16 ++++-------- app/controllers/comment_votes_controller.rb | 1 - app/controllers/comments_controller.rb | 1 - app/controllers/favorites_controller.rb | 1 - app/controllers/forum_posts_controller.rb | 1 - app/controllers/forum_topics_controller.rb | 1 - .../moderator/post/posts_controller.rb | 1 - .../post_disapprovals_controller.rb | 1 - app/controllers/post_votes_controller.rb | 1 - app/controllers/users_controller.rb | 1 - app/logical/rate_limiter.rb | 22 ++++++++++++++++ app/models/user.rb | 20 +-------------- app/policies/user_policy.rb | 6 ++--- test/functional/sessions_controller_test.rb | 25 +++++++++++++++++++ test/unit/api_key_test.rb | 6 ----- 15 files changed, 55 insertions(+), 49 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8c8237bdd..af9d36964 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -8,7 +8,7 @@ class ApplicationController < ActionController::Base before_action :reset_current_user before_action :set_current_user before_action :normalize_search - before_action :api_check + before_action :check_rate_limit before_action :ip_ban_check before_action :set_variant before_action :add_headers @@ -69,18 +69,12 @@ class ApplicationController < ActionController::Base response.headers["X-Git-Hash"] = Rails.application.config.x.git_hash end - def api_check - return if CurrentUser.is_anonymous? || request.get? || request.head? - - rate_limiter = RateLimiter.new( - "write", - [CurrentUser.user.cache_key], - cost: 1, - rate: CurrentUser.user.api_regen_multiplier, - burst: CurrentUser.user.api_burst_limit - ) + def check_rate_limit + return if request.get? || request.head? + rate_limiter = RateLimiter.for_action(controller_name, action_name, CurrentUser.user, CurrentUser.ip_addr) headers["X-Rate-Limit"] = rate_limiter.to_json + rate_limiter.limit! end diff --git a/app/controllers/comment_votes_controller.rb b/app/controllers/comment_votes_controller.rb index c85faf4d8..8e280385a 100644 --- a/app/controllers/comment_votes_controller.rb +++ b/app/controllers/comment_votes_controller.rb @@ -1,5 +1,4 @@ class CommentVotesController < ApplicationController - skip_before_action :api_check respond_to :js, :json, :xml, :html def index diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index f655385ce..54783e0c1 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -1,7 +1,6 @@ class CommentsController < ApplicationController respond_to :html, :xml, :json, :atom respond_to :js, only: [:new, :destroy, :undelete] - skip_before_action :api_check def index params[:group_by] ||= "comment" if params[:search].present? diff --git a/app/controllers/favorites_controller.rb b/app/controllers/favorites_controller.rb index 8449dac63..2f086b6b8 100644 --- a/app/controllers/favorites_controller.rb +++ b/app/controllers/favorites_controller.rb @@ -1,6 +1,5 @@ class FavoritesController < ApplicationController respond_to :html, :xml, :json, :js - skip_before_action :api_check rescue_with Favorite::Error, status: 422 def index diff --git a/app/controllers/forum_posts_controller.rb b/app/controllers/forum_posts_controller.rb index 53af4054b..0a239591c 100644 --- a/app/controllers/forum_posts_controller.rb +++ b/app/controllers/forum_posts_controller.rb @@ -1,6 +1,5 @@ class ForumPostsController < ApplicationController respond_to :html, :xml, :json, :js - skip_before_action :api_check def new @forum_post = authorize ForumPost.new_reply(params) diff --git a/app/controllers/forum_topics_controller.rb b/app/controllers/forum_topics_controller.rb index f9a0ef00c..03865d1ae 100644 --- a/app/controllers/forum_topics_controller.rb +++ b/app/controllers/forum_topics_controller.rb @@ -2,7 +2,6 @@ class ForumTopicsController < ApplicationController respond_to :html, :xml, :json respond_to :atom, only: [:index, :show] before_action :normalize_search, :only => :index - skip_before_action :api_check def new @forum_topic = authorize ForumTopic.new diff --git a/app/controllers/moderator/post/posts_controller.rb b/app/controllers/moderator/post/posts_controller.rb index bc0603c16..ab28bd2f5 100644 --- a/app/controllers/moderator/post/posts_controller.rb +++ b/app/controllers/moderator/post/posts_controller.rb @@ -1,7 +1,6 @@ module Moderator module Post class PostsController < ApplicationController - skip_before_action :api_check respond_to :html, :json, :xml, :js def confirm_move_favorites diff --git a/app/controllers/post_disapprovals_controller.rb b/app/controllers/post_disapprovals_controller.rb index 6dbd93f46..e27853ee9 100644 --- a/app/controllers/post_disapprovals_controller.rb +++ b/app/controllers/post_disapprovals_controller.rb @@ -1,5 +1,4 @@ class PostDisapprovalsController < ApplicationController - skip_before_action :api_check respond_to :js, :html, :json, :xml def create diff --git a/app/controllers/post_votes_controller.rb b/app/controllers/post_votes_controller.rb index 1c8f8ff2f..1f66f4cf3 100644 --- a/app/controllers/post_votes_controller.rb +++ b/app/controllers/post_votes_controller.rb @@ -1,5 +1,4 @@ class PostVotesController < ApplicationController - skip_before_action :api_check respond_to :js, :json, :xml, :html def index diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 387037ea1..acd68815d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,6 +1,5 @@ class UsersController < ApplicationController respond_to :html, :xml, :json - skip_before_action :api_check def new @user = authorize User.new diff --git a/app/logical/rate_limiter.rb b/app/logical/rate_limiter.rb index e8fc52430..56851f282 100644 --- a/app/logical/rate_limiter.rb +++ b/app/logical/rate_limiter.rb @@ -11,6 +11,28 @@ class RateLimiter @burst = burst end + def self.for_action(controller_name, action_name, user, ip_addr) + action = "#{controller_name}:#{action_name}" + keys = [(user.cache_key unless user.is_anonymous?), "ip/#{ip_addr.to_s}"].compact + + case action + when "users:create" + rate, burst = 1.0/5.minutes, 10 + when "emails:update", "sessions:create", "moderation_reports:create" + rate, burst = 1.0/1.minute, 10 + when "dmails:create", "comments:create", "forum_posts:create", "forum_topics:create" + rate, burst = 1.0/1.minute, 50 + when "comment_votes:create", "comment_votes:destroy", "post_votes:create", + "post_votes:destroy", "favorites:create", "favorites:destroy", "post_disapprovals:create" + rate, burst = 1.0/1.second, 200 + else + rate = user.api_regen_multiplier + burst = 200 + end + + RateLimiter.new(action, keys, rate: rate, burst: burst) + end + def limit! raise RateLimitError if limited? end diff --git a/app/models/user.rb b/app/models/user.rb index 22dfa82b1..03226fc26 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -464,26 +464,12 @@ class User < ApplicationRecord # regen this amount per second def api_regen_multiplier(level) - if level >= User::Levels::PLATINUM + if level >= User::Levels::GOLD 4 - elsif level == User::Levels::GOLD - 2 else 1 end end - - # can make this many api calls at once before being bound by - # api_regen_multiplier refilling your pool - def api_burst_limit(level) - if level >= User::Levels::PLATINUM - 60 - elsif level == User::Levels::GOLD - 30 - else - 10 - end - end end def max_saved_searches @@ -534,10 +520,6 @@ class User < ApplicationRecord User.api_regen_multiplier(level) end - def api_burst_limit - User.api_burst_limit(level) - end - def statement_timeout User.statement_timeout(level) end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index b5bef3ba9..fd6f93d06 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -61,10 +61,8 @@ class UserPolicy < ApplicationPolicy updated_at last_logged_in_at last_forum_read_at comment_threshold default_image_size favorite_tags blacklisted_tags time_zone per_page - custom_style favorite_count api_regen_multiplier - api_burst_limit statement_timeout - favorite_group_limit favorite_limit tag_query_limit - max_saved_searches theme + custom_style favorite_count statement_timeout favorite_group_limit + favorite_limit tag_query_limit max_saved_searches theme ] end diff --git a/test/functional/sessions_controller_test.rb b/test/functional/sessions_controller_test.rb index 29b3dbff6..eb9ab6fb6 100644 --- a/test/functional/sessions_controller_test.rb +++ b/test/functional/sessions_controller_test.rb @@ -72,6 +72,31 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest assert_equal(0, @ip_ban.reload.hit_count) assert_nil(@ip_ban.last_hit_at) end + + should "rate limit logins to 10 per minute per IP" do + freeze_time + + 11.times do + post session_path, params: { name: @user.name, password: "password" }, headers: { REMOTE_ADDR: "1.2.3.4" } + assert_redirected_to posts_path + assert_equal(@user.id, session[:user_id]) + delete_auth session_path, @user + end + + post session_path, params: { name: @user.name, password: "password" }, headers: { REMOTE_ADDR: "1.2.3.4" } + assert_response 429 + assert_not_equal(@user.id, session[:user_id]) + + travel 59.seconds + post session_path, params: { name: @user.name, password: "password" }, headers: { REMOTE_ADDR: "1.2.3.4" } + assert_response 429 + assert_not_equal(@user.id, session[:user_id]) + + travel 10.seconds + post session_path, params: { name: @user.name, password: "password" }, headers: { REMOTE_ADDR: "1.2.3.4" } + assert_redirected_to posts_path + assert_equal(@user.id, session[:user_id]) + end end context "destroy action" do diff --git a/test/unit/api_key_test.rb b/test/unit/api_key_test.rb index 26b68a67f..760035ae2 100644 --- a/test/unit/api_key_test.rb +++ b/test/unit/api_key_test.rb @@ -48,11 +48,5 @@ class ApiKeyTest < ActiveSupport::TestCase should "not authenticate with the wrong name" do assert_equal(false, create(:user).authenticate_api_key(@api_key.key)) end - - should "have the same limits whether or not they have an api key" do - assert_no_difference(["@user.reload.api_regen_multiplier", "@user.reload.api_burst_limit"]) do - @api_key.destroy - end - end end end From 1ee1e807cf2a25067a98860bb413d5d931a5951f Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 5 Mar 2021 15:53:03 -0600 Subject: [PATCH 07/27] rate limits: penalize user if they keep making requests while limited. If the user makes a request while rate limited, penalize them 1 second for that request, up to a maximum of 30 seconds. This means that if a user doesn't stop making requests after being rate limited, then they will stay rate limited forever until they stop. This is to temp ban bots, especially spam bots, that flood requests while ignoring HTTP errors or rate limits. (Note that this is on a per-endpoint basis. Being rate limited on one endpoint won't penalize you for making calls to other endpoints.) --- app/models/rate_limit.rb | 13 ++++++++++--- test/unit/rate_limit_test.rb | 10 +++++----- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/app/models/rate_limit.rb b/app/models/rate_limit.rb index 3236f1ac4..b3d0b8ab0 100644 --- a/app/models/rate_limit.rb +++ b/app/models/rate_limit.rb @@ -10,7 +10,7 @@ class RateLimit < ApplicationRecord # `cost` is the number of points the action costs. # `rate` is the number of points per second that are refilled. # `burst` is the maximum number of points that can be saved up. - def self.create_or_update!(action:, keys:, cost:, rate:, burst:) + def self.create_or_update!(action:, keys:, cost:, rate:, burst:, minimum_points: -30) # { key0: keys[0], ..., keyN: keys[N] } key_params = keys.map.with_index { |key, i| [:"key#{i}", key] }.to_h @@ -20,6 +20,12 @@ class RateLimit < ApplicationRecord # Do an upsert, creating a new rate limit object for each key that doesn't # already exist, and updating the limit for each limit that already exists. # + # If the current point count is negative, then we're limited. Penalize the + # caller 1 second (1 rate unit), up to a maximum penalty of 30 seconds (by default). + # + # Otherwise, if the point count is positive, then we're not limited. Update + # the point count and subtract the cost of the call. + # # https://www.postgresql.org/docs/current/sql-insert.html#SQL-ON-CONFLICT sql = <<~SQL INSERT INTO rate_limits (created_at, updated_at, action, key, points) @@ -30,9 +36,9 @@ class RateLimit < ApplicationRecord points = CASE WHEN rate_limits.points + :rate * EXTRACT(epoch FROM (:now - rate_limits.updated_at)) < 0 THEN - LEAST(:burst, rate_limits.points + :rate * EXTRACT(epoch FROM (:now - rate_limits.updated_at))) + GREATEST(:minimum_points, LEAST(:burst, rate_limits.points + :rate * EXTRACT(epoch FROM (:now - rate_limits.updated_at))) - :rate) ELSE - LEAST(:burst, rate_limits.points + :rate * EXTRACT(epoch FROM (:now - rate_limits.updated_at))) - :cost + GREATEST(:minimum_points, LEAST(:burst, rate_limits.points + :rate * EXTRACT(epoch FROM (:now - rate_limits.updated_at))) - :cost) END RETURNING * SQL @@ -44,6 +50,7 @@ class RateLimit < ApplicationRecord burst: burst, cost: cost, points: burst - cost, + minimum_points: minimum_points, **key_params } diff --git a/test/unit/rate_limit_test.rb b/test/unit/rate_limit_test.rb index d120a494a..6a8a47725 100644 --- a/test/unit/rate_limit_test.rb +++ b/test/unit/rate_limit_test.rb @@ -26,13 +26,13 @@ class RateLimitTest < ActiveSupport::TestCase assert_equal(9, limiter.rate_limits.first.points) end - should "be limited if the point count is negative" do + should "be limited and penalize the caller 1 second if the point count is negative" do freeze_time create(:rate_limit, action: "write", key: "users/1", points: -1) limiter = RateLimiter.new("write", ["users/1"], cost: 1) assert_equal(true, limiter.limited?) - assert_equal(-1, limiter.rate_limits.first.points) + assert_equal(-2, limiter.rate_limits.first.points) end should "not be limited if the point count was positive before the action" do @@ -50,17 +50,17 @@ class RateLimitTest < ActiveSupport::TestCase limiter = RateLimiter.new("write", ["users/1"], cost: 1, rate: 1, burst: 10) assert_equal(true, limiter.limited?) - assert_equal(-2, limiter.rate_limits.first.points) + assert_equal(-3, limiter.rate_limits.first.points) travel 1.second limiter = RateLimiter.new("write", ["users/1"], cost: 1, rate: 1, burst: 10) assert_equal(true, limiter.limited?) - assert_equal(-1, limiter.rate_limits.first.points) + assert_equal(-3, limiter.rate_limits.first.points) travel 5.second limiter = RateLimiter.new("write", ["users/1"], cost: 1, rate: 1, burst: 10) assert_equal(false, limiter.limited?) - assert_equal(3, limiter.rate_limits.first.points) + assert_equal(1, limiter.rate_limits.first.points) travel 60.second limiter = RateLimiter.new("write", ["users/1"], cost: 1, rate: 1, burst: 10) From 58e42ee8d3f052a6b3b15bdef75f5f7f9e7069af Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 5 Mar 2021 16:25:23 -0600 Subject: [PATCH 08/27] rate limits: add /rate_limits endpoint. Allow users to view their own rate limits with /rate_limits.json. Note that rate limits are only updated after every API call, so this page only shows the state of the limits after the last call, not the current state. --- app/controllers/rate_limits_controller.rb | 8 +++++ app/models/rate_limit.rb | 16 +++++++++ app/policies/rate_limit_policy.rb | 5 +++ app/views/rate_limits/index.html.erb | 21 ++++++++++++ config/routes.rb | 1 + .../functional/rate_limits_controller_test.rb | 33 +++++++++++++++++++ 6 files changed, 84 insertions(+) create mode 100644 app/controllers/rate_limits_controller.rb create mode 100644 app/policies/rate_limit_policy.rb create mode 100644 app/views/rate_limits/index.html.erb create mode 100644 test/functional/rate_limits_controller_test.rb diff --git a/app/controllers/rate_limits_controller.rb b/app/controllers/rate_limits_controller.rb new file mode 100644 index 000000000..daaa8d899 --- /dev/null +++ b/app/controllers/rate_limits_controller.rb @@ -0,0 +1,8 @@ +class RateLimitsController < ApplicationController + respond_to :html, :json, :xml + + def index + @rate_limits = authorize RateLimit.visible(CurrentUser.user).paginated_search(params, count_pages: true) + respond_with(@rate_limits) + end +end diff --git a/app/models/rate_limit.rb b/app/models/rate_limit.rb index b3d0b8ab0..dc9daee90 100644 --- a/app/models/rate_limit.rb +++ b/app/models/rate_limit.rb @@ -5,6 +5,22 @@ class RateLimit < ApplicationRecord expired.delete_all end + def self.visible(user) + if user.is_owner? + all + elsif user.is_anonymous? + none + else + where(key: [user.cache_key]) + end + end + + def self.search(params) + q = search_attributes(params, :id, :created_at, :updated_at, :limited, :points, :action, :key) + q = q.apply_default_order(params) + q + end + # `action` is the action being limited. Usually a controller endpoint. # `keys` is who is being limited. Usually a [user, ip] pair, meaning the action is limited both by the user's ID and their IP. # `cost` is the number of points the action costs. diff --git a/app/policies/rate_limit_policy.rb b/app/policies/rate_limit_policy.rb new file mode 100644 index 000000000..9970ac553 --- /dev/null +++ b/app/policies/rate_limit_policy.rb @@ -0,0 +1,5 @@ +class RateLimitPolicy < ApplicationPolicy + def index? + true + end +end diff --git a/app/views/rate_limits/index.html.erb b/app/views/rate_limits/index.html.erb new file mode 100644 index 000000000..4ac3ffcc0 --- /dev/null +++ b/app/views/rate_limits/index.html.erb @@ -0,0 +1,21 @@ +
+
+ <%= table_for @rate_limits, class: "striped autofit" do |t| %> + <% t.column :action %> + + <% t.column :key %> + + <% t.column :points do |rate_limit| %> + <%= rate_limit.points.round(2) %> + <% end %> + + <% t.column :limited? %> + + <% t.column :updated_at do |rate_limit| %> + <%= time_ago_in_words_tagged rate_limit.updated_at %> + <% end %> + <% end %> + + <%= numbered_paginator(@rate_limits) %> +
+
diff --git a/config/routes.rb b/config/routes.rb index 61010d9ed..82df33b1c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -220,6 +220,7 @@ Rails.application.routes.draw do end end resources :artist_commentary_versions, :only => [:index, :show] + resources :rate_limits, only: [:index] resource :related_tag, :only => [:show, :update] resources :recommended_posts, only: [:index] resources :robots, only: [:index] diff --git a/test/functional/rate_limits_controller_test.rb b/test/functional/rate_limits_controller_test.rb new file mode 100644 index 000000000..499a018cc --- /dev/null +++ b/test/functional/rate_limits_controller_test.rb @@ -0,0 +1,33 @@ +require 'test_helper' + +class RateLimitsControllerTest < ActionDispatch::IntegrationTest + context "The rate limits controller" do + context "index action" do + setup do + @user = create(:user) + create(:rate_limit, key: @user.cache_key) + end + + should "show all rate limits to the owner" do + get_auth rate_limits_path, create(:owner_user) + + assert_response :success + assert_select "tbody tr", count: 2 # 2 because the login action creates a second rate limit. + end + + should "show the user their own rate limits" do + get_auth rate_limits_path, @user + + assert_response :success + assert_select "tbody tr", count: 1 + end + + should "not show users rate limits belonging to other users" do + get_auth rate_limits_path, create(:user) + + assert_response :success + assert_select "tbody tr", count: 0 + end + end + end +end From be162a8ae96433cfae4c7231b5191eddfacc60e9 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 5 Mar 2021 18:22:47 -0600 Subject: [PATCH 09/27] Fix #4746: Related tags checkboxes don't work properly on some phones (iOS). Only use hover to hide the checkboxes on devices that support hovering (i.e. computers with a mouse). On some mobile devices, a tap is used to emulate hovering, which meant the tag had to be tapped twice. --- .../src/styles/specific/related_tags.scss | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/app/javascript/src/styles/specific/related_tags.scss b/app/javascript/src/styles/specific/related_tags.scss index 03930c453..66cc8dd38 100644 --- a/app/javascript/src/styles/specific/related_tags.scss +++ b/app/javascript/src/styles/specific/related_tags.scss @@ -15,11 +15,6 @@ div.related-tags { width: 15em; } - /* Hide the related tag checkbox unless it's checked or hovered. */ - input[type="checkbox"] { - visibility: hidden; - } - li.selected a { /* https://developer.mozilla.org/en-US/docs/Web/CSS/-webkit-text-stroke */ /* https://caniuse.com/text-stroke */ @@ -27,7 +22,14 @@ div.related-tags { text-stroke: 0.5px; } - li.selected, li:hover { - input[type="checkbox"] { visibility: visible; } + /* On computers with a mouse, hide the related tag checkbox unless it's checked or hovered. */ + @media (hover: hover) { + input[type="checkbox"] { + visibility: hidden; + } + + li.selected, li:hover { + input[type="checkbox"] { visibility: visible; } + } } } From bb0540e1a18f32dd09404a5081903b4fe075d026 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 5 Mar 2021 19:53:42 -0600 Subject: [PATCH 10/27] Fix #4747: BUR layout partly broken on iOS. Move the BUR help text from the