From 0c7c188c459117770ac1f74f70a9e65ed2be606f Mon Sep 17 00:00:00 2001 From: Sorin Davidoi Date: Thu, 13 Jul 2017 22:15:32 +0200 Subject: Web Push Notifications (#3243) * feat: Register push subscription * feat: Notify when mentioned * feat: Boost, favourite, reply, follow, follow request * feat: Notification interaction * feat: Handle change of public key * feat: Unsubscribe if things go wrong * feat: Do not send normal notifications if push is enabled * feat: Focus client if open * refactor: Move push logic to WebPushSubscription * feat: Better title and body * feat: Localize messages * chore: Fix lint errors * feat: Settings * refactor: Lazy load * fix: Check if push settings exist * feat: Device-based preferences * refactor: Simplify logic * refactor: Pull request feedback * refactor: Pull request feedback * refactor: Create /api/web/push_subscriptions endpoint * feat: Spec PushSubscriptionController * refactor: WebPushSubscription => Web::PushSubscription * feat: Spec Web::PushSubscription * feat: Display first media attachment * feat: Support direction * fix: Stuff broken while rebasing * refactor: Integration with session activations * refactor: Cleanup * refactor: Simplify implementation * feat: Set VAPID keys via environment * chore: Comments * fix: Crash when no alerts * fix: Set VAPID keys in testing environment * fix: Follow link * feat: Notification actions * fix: Delete previous subscription * chore: Temporary logs * refactor: Move migration to a later date * fix: Fetch the correct session activation and misc bugs * refactor: Move migration to a later date * fix: Remove follow request (no notifications) * feat: Send administrator contact to push service * feat: Set time-to-live * fix: Do not show sensitive images * fix: Reducer crash in error handling * feat: Add badge * chore: Fix lint error * fix: Checkbox label overlap * fix: Check for payload support * fix: Rename action "type" (crash in latest Chrome) * feat: Action to expand notification * fix: Lint errors * fix: Unescape notification body * fix: Do not allow boosting if the status is hidden * feat: Add VAPID keys to the production sample environment * fix: Strip HTML tags from status * refactor: Better error messages * refactor: Handle browser not implementing the VAPID protocol (Samsung Internet) * fix: Error when target_status is nil * fix: Handle lack of image * fix: Delete reference to invalid subscriptions * feat: Better error handling * fix: Unescape HTML characters after tags are striped * refactor: Simpify code * fix: Modify to work with #4091 * Sort strings alphabetically * i18n: Updated Polish translation it annoys me that it's not fully localized :P * refactor: Use current_session in PushSubscriptionController * fix: Rebase mistake * fix: Set cacheName to mastodon * refactor: Pull request feedback * refactor: Remove logging statements * chore(yarn): Fix conflicts with master * chore(yarn): Copy latest from master * chore(yarn): Readd offline-plugin * refactor: Use save! and update! * refactor: Send notifications async * fix: Allow retry when push fails * fix: Save track for failed pushes * fix: Minify sw.js * fix: Remove account_id from fabricator --- app/services/notify_service.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'app/services') diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index 407d385ea..0ab61b634 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -61,6 +61,11 @@ class NotifyService < BaseService @notification.save! return unless @notification.browserable? Redis.current.publish("timeline:#{@recipient.id}", Oj.dump(event: :notification, payload: InlineRenderer.render(@notification, @recipient, :notification))) + send_push_notifications + end + + def send_push_notifications + WebPushNotificationWorker.perform_async(@recipient.id, @notification.id) end def send_email -- cgit From e2685ccc81f04e1a63a97af80686bf85027418a6 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 14 Jul 2017 19:47:53 +0200 Subject: Fix #4149, fix #1199 - Store emojis as unicode (#4189) - Use unicode when selecting emoji through picker - Convert shortcodes to unicode when storing text input server-side - Do not convert shortcodes in JS anymore --- Gemfile | 1 + Gemfile.lock | 4 ++- app/helpers/emoji_helper.rb | 19 ++++++++++++ app/javascript/mastodon/actions/compose.js | 7 +++-- app/javascript/mastodon/emoji.js | 24 ++------------- .../features/compose/components/compose_form.js | 3 +- .../compose/components/emoji_picker_dropdown.js | 5 +-- app/javascript/mastodon/reducers/compose.js | 2 +- app/javascript/styles/components.scss | 1 + app/models/account.rb | 10 ++++++ app/models/status.rb | 6 +++- app/services/post_status_service.rb | 2 ++ spec/helpers/emoji_helper_spec.rb | 15 +++++++++ spec/helpers/routing_helper.rb | 5 --- spec/javascript/components/emojify.test.js | 36 ---------------------- 15 files changed, 69 insertions(+), 71 deletions(-) create mode 100644 app/helpers/emoji_helper.rb create mode 100644 spec/helpers/emoji_helper_spec.rb delete mode 100644 spec/helpers/routing_helper.rb (limited to 'app/services') diff --git a/Gemfile b/Gemfile index 988b4d6b9..531d01ae0 100644 --- a/Gemfile +++ b/Gemfile @@ -28,6 +28,7 @@ gem 'devise', '~> 4.2' gem 'devise-two-factor', '~> 3.0' gem 'doorkeeper', '~> 4.2' gem 'fast_blank', '~> 1.0' +gem 'gemoji', '~> 3.0' gem 'goldfinger', '~> 1.2' gem 'hiredis', '~> 0.6' gem 'redis-namespace', '~> 1.5' diff --git a/Gemfile.lock b/Gemfile.lock index 5599e1db1..83202189d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -106,9 +106,9 @@ GEM rack (>= 1.0.0) rack-test (>= 0.5.4) xpath (~> 2.0) - charlock_holmes (0.7.3) case_transform (0.2) activesupport + charlock_holmes (0.7.3) chunky_png (1.3.8) cld3 (3.1.3) ffi (>= 1.1.0, < 1.10.0) @@ -163,6 +163,7 @@ GEM fuubar (2.2.0) rspec-core (~> 3.0) ruby-progressbar (~> 1.4) + gemoji (3.0.0) globalid (0.4.0) activesupport (>= 4.2.0) goldfinger (1.2.0) @@ -518,6 +519,7 @@ DEPENDENCIES faker (~> 1.7) fast_blank (~> 1.0) fuubar (~> 2.2) + gemoji (~> 3.0) goldfinger (~> 1.2) hamlit-rails (~> 0.2) hiredis (~> 0.6) diff --git a/app/helpers/emoji_helper.rb b/app/helpers/emoji_helper.rb new file mode 100644 index 000000000..c1595851f --- /dev/null +++ b/app/helpers/emoji_helper.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module EmojiHelper + EMOJI_PATTERN = /(?<=[^[:alnum:]:]|\n|^):([\w+-]+):(?=[^[:alnum:]:]|$)/x + + def emojify(text) + return text if text.blank? + + text.gsub(EMOJI_PATTERN) do |match| + emoji = Emoji.find_by_alias($1) # rubocop:disable Rails/DynamicFindBy,Style/PerlBackrefs + + if emoji + emoji.raw + else + match + end + end + end +end diff --git a/app/javascript/mastodon/actions/compose.js b/app/javascript/mastodon/actions/compose.js index 647a52b93..9f05a53e9 100644 --- a/app/javascript/mastodon/actions/compose.js +++ b/app/javascript/mastodon/actions/compose.js @@ -2,8 +2,6 @@ import api from '../api'; import { updateTimeline } from './timelines'; -import * as emojione from 'emojione'; - export const COMPOSE_CHANGE = 'COMPOSE_CHANGE'; export const COMPOSE_SUBMIT_REQUEST = 'COMPOSE_SUBMIT_REQUEST'; export const COMPOSE_SUBMIT_SUCCESS = 'COMPOSE_SUBMIT_SUCCESS'; @@ -73,11 +71,14 @@ export function mentionCompose(account, router) { export function submitCompose() { return function (dispatch, getState) { - const status = emojione.shortnameToUnicode(getState().getIn(['compose', 'text'], '')); + const status = getState().getIn(['compose', 'text'], ''); + if (!status || !status.length) { return; } + dispatch(submitComposeRequest()); + api(getState).post('/api/v1/statuses', { status, in_reply_to_id: getState().getIn(['compose', 'in_reply_to'], null), diff --git a/app/javascript/mastodon/emoji.js b/app/javascript/mastodon/emoji.js index 7043d5f3a..ed2180cd1 100644 --- a/app/javascript/mastodon/emoji.js +++ b/app/javascript/mastodon/emoji.js @@ -6,36 +6,18 @@ const trie = new Trie(Object.keys(emojione.jsEscapeMap)); function emojify(str) { // This walks through the string from start to end, ignoring any tags (

,
, etc.) - // and replacing valid shortnames like :smile: and :wink: as well as unicode strings + // and replacing valid unicode strings // that _aren't_ within tags with an version. - // The goal is to be the same as an emojione.regShortNames/regUnicode replacement, but faster. + // The goal is to be the same as an emojione.regUnicode replacement, but faster. let i = -1; let insideTag = false; - let insideShortname = false; - let shortnameStartIndex = -1; let match; while (++i < str.length) { const char = str.charAt(i); - if (insideShortname && char === ':') { - const shortname = str.substring(shortnameStartIndex, i + 1); - if (shortname in emojione.emojioneList) { - const unicode = emojione.emojioneList[shortname].unicode[emojione.emojioneList[shortname].unicode.length - 1]; - const alt = emojione.convert(unicode.toUpperCase()); - const replacement = `${alt}`; - str = str.substring(0, shortnameStartIndex) + replacement + str.substring(i + 1); - i += (replacement.length - shortname.length - 1); // jump ahead the length we've added to the string - } else { - i--; // stray colon, try again - } - insideShortname = false; - } else if (insideTag && char === '>') { + if (insideTag && char === '>') { insideTag = false; } else if (char === '<') { insideTag = true; - insideShortname = false; - } else if (!insideTag && char === ':') { - insideShortname = true; - shortnameStartIndex = i; } else if (!insideTag && (match = trie.search(str.substring(i)))) { const unicodeStr = match; if (unicodeStr in emojione.jsEscapeMap) { diff --git a/app/javascript/mastodon/features/compose/components/compose_form.js b/app/javascript/mastodon/features/compose/components/compose_form.js index f7eeedc69..f07552947 100644 --- a/app/javascript/mastodon/features/compose/components/compose_form.js +++ b/app/javascript/mastodon/features/compose/components/compose_form.js @@ -136,7 +136,8 @@ export default class ComposeForm extends ImmutablePureComponent { handleEmojiPick = (data) => { const position = this.autosuggestTextarea.textarea.selectionStart; - this._restoreCaret = position + data.shortname.length + 1; + const emojiChar = String.fromCodePoint(parseInt(data.unicode, 16)); + this._restoreCaret = position + emojiChar.length + 1; this.props.onPickEmoji(position, data); } diff --git a/app/javascript/mastodon/features/compose/components/emoji_picker_dropdown.js b/app/javascript/mastodon/features/compose/components/emoji_picker_dropdown.js index 83c66a5d5..acc584f20 100644 --- a/app/javascript/mastodon/features/compose/components/emoji_picker_dropdown.js +++ b/app/javascript/mastodon/features/compose/components/emoji_picker_dropdown.js @@ -109,11 +109,12 @@ export default class EmojiPickerDropdown extends React.PureComponent { 🙂 + { this.state.active && !this.state.loading && diff --git a/app/javascript/mastodon/reducers/compose.js b/app/javascript/mastodon/reducers/compose.js index a92b5aa23..ea3b78b67 100644 --- a/app/javascript/mastodon/reducers/compose.js +++ b/app/javascript/mastodon/reducers/compose.js @@ -118,7 +118,7 @@ const insertSuggestion = (state, position, token, completion) => { }; const insertEmoji = (state, position, emojiData) => { - const emoji = emojiData.shortname; + const emoji = String.fromCodePoint(parseInt(emojiData.unicode, 16)); return state.withMutations(map => { map.update('text', oldText => `${oldText.slice(0, position)}${emoji} ${oldText.slice(position)}`); diff --git a/app/javascript/styles/components.scss b/app/javascript/styles/components.scss index 0face646d..0420a2bed 100644 --- a/app/javascript/styles/components.scss +++ b/app/javascript/styles/components.scss @@ -2708,6 +2708,7 @@ button.icon-button.active i.fa-retweet { margin-left: 2px; width: 24px; outline: 0; + cursor: pointer; &:active, &:focus { diff --git a/app/models/account.rb b/app/models/account.rb index 2b54cee5f..7243cb1a5 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -47,6 +47,7 @@ class Account < ApplicationRecord include AccountInteractions include Attachmentable include Remotable + include EmojiHelper # Local users has_one :user, inverse_of: :account @@ -240,9 +241,18 @@ class Account < ApplicationRecord before_create :generate_keys before_validation :normalize_domain + before_validation :prepare_contents, if: :local? private + def prepare_contents + display_name&.strip! + note&.strip! + + self.display_name = emojify(display_name) + self.note = emojify(note) + end + def generate_keys return unless local? diff --git a/app/models/status.rb b/app/models/status.rb index 65db7579a..24eaf7071 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -29,6 +29,7 @@ class Status < ApplicationRecord include Streamable include Cacheable include StatusThreadingConcern + include EmojiHelper enum visibility: [:public, :unlisted, :private, :direct], _suffix: :visibility @@ -120,7 +121,7 @@ class Status < ApplicationRecord !sensitive? && media_attachments.any? end - before_validation :prepare_contents + before_validation :prepare_contents, if: :local? before_validation :set_reblog before_validation :set_visibility before_validation :set_conversation @@ -241,6 +242,9 @@ class Status < ApplicationRecord def prepare_contents text&.strip! spoiler_text&.strip! + + self.text = emojify(text) + self.spoiler_text = emojify(spoiler_text) end def set_reblog diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 2e6fbb5c3..951a38e19 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -21,6 +21,7 @@ class PostStatusService < BaseService media = validate_media!(options[:media_ids]) status = nil + ApplicationRecord.transaction do status = account.statuses.create!(text: text, thread: in_reply_to, @@ -31,6 +32,7 @@ class PostStatusService < BaseService application: options[:application]) attach_media(status, media) end + process_mentions_service.call(status) process_hashtags_service.call(status) diff --git a/spec/helpers/emoji_helper_spec.rb b/spec/helpers/emoji_helper_spec.rb new file mode 100644 index 000000000..1eedfb719 --- /dev/null +++ b/spec/helpers/emoji_helper_spec.rb @@ -0,0 +1,15 @@ +require 'rails_helper' + +RSpec.describe EmojiHelper, type: :helper do + describe '#emojify' do + it 'converts shortcodes to unicode' do + text = ':book: Book' + expect(emojify(text)).to eq '📖 Book' + end + + it 'does not convert shortcodes that are part of a string into unicode' do + text = ':see_no_evil::hear_no_evil::speak_no_evil:' + expect(emojify(text)).to eq text + end + end +end diff --git a/spec/helpers/routing_helper.rb b/spec/helpers/routing_helper.rb deleted file mode 100644 index 3cd397397..000000000 --- a/spec/helpers/routing_helper.rb +++ /dev/null @@ -1,5 +0,0 @@ -require 'rails_helper' - -RSpec.describe RoutingHelper, type: :helper do - -end diff --git a/spec/javascript/components/emojify.test.js b/spec/javascript/components/emojify.test.js index 3e8b25af9..e165b4519 100644 --- a/spec/javascript/components/emojify.test.js +++ b/spec/javascript/components/emojify.test.js @@ -2,32 +2,6 @@ import { expect } from 'chai'; import emojify from '../../../app/javascript/mastodon/emoji'; describe('emojify', () => { - it('does a basic emojify', () => { - expect(emojify(':smile:')).to.equal( - '😄'); - }); - - it('does a double emojify', () => { - expect(emojify(':smile: and :wink:')).to.equal( - '😄 and 😉'); - }); - - it('works with random colons', () => { - expect(emojify(':smile: : :wink:')).to.equal( - '😄 : 😉'); - expect(emojify(':smile::::wink:')).to.equal( - '😄::😉'); - expect(emojify(':smile:::::wink:')).to.equal( - '😄:::😉'); - }); - - it('works with tags', () => { - expect(emojify('

:smile:

')).to.equal( - '

😄

'); - expect(emojify('

:smile:

and

:wink:

')).to.equal( - '

😄

and

😉

'); - }); - it('ignores unknown shortcodes', () => { expect(emojify(':foobarbazfake:')).to.equal(':foobarbazfake:'); }); @@ -46,11 +20,6 @@ describe('emojify', () => { expect(emojify(':smile')).to.equal(':smile'); }); - it('does two emoji next to each other', () => { - expect(emojify(':smile::wink:')).to.equal( - '😄😉'); - }); - it('does unicode', () => { expect(emojify('\uD83D\uDC69\u200D\uD83D\uDC69\u200D\uD83D\uDC66\u200D\uD83D\uDC66')).to.equal( '👩‍👩‍👦‍👦'); @@ -72,12 +41,7 @@ describe('emojify', () => { 'foo ❗ #️⃣ bar'); }); - it('does mixed unicode and shortnames', () => { - expect(emojify(':smile:#\uFE0F\u20E3:wink:\u2757')).to.equal('😄#️⃣😉❗'); - }); - it('ignores unicode inside of tags', () => { expect(emojify('

')).to.equal('

'); }); - }); -- cgit From 1618b68bfa740ed655ac45d7d5f4f46fed6c8c62 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 14 Jul 2017 20:41:49 +0200 Subject: HTTP signatures (#4146) * Add Request class with HTTP signature generator Spec: https://tools.ietf.org/html/draft-cavage-http-signatures-06 * Add HTTP signature verification concern * Add test for SignatureVerification concern * Add basic test for Request class * Make PuSH subscribe/unsubscribe requests use new Request class Accidentally fix lease_seconds not being set and sent properly, and change the new minimum subscription duration to 1 day * Make all PuSH workers use new Request class * Make Salmon sender use new Request class * Make FetchLinkService use new Request class * Make FetchAtomService use the new Request class * Make Remotable use the new Request class * Make ResolveRemoteAccountService use the new Request class * Add more tests * Allow +-30 seconds window for signed request to remain valid * Disable time window validation for signed requests, restore 7 days as PuSH subscription duration (which was previous default due to a bug) --- app/controllers/accounts_controller.rb | 1 + app/controllers/api/subscriptions_controller.rb | 2 +- app/controllers/concerns/signature_verification.rb | 87 ++++++++++++++++++++++ app/controllers/stream_entries_controller.rb | 1 + app/helpers/http_helper.rb | 17 ----- app/lib/provider_discovery.rb | 4 +- app/lib/request.rb | 70 +++++++++++++++++ app/models/account.rb | 2 +- app/models/concerns/remotable.rb | 3 +- app/models/subscription.rb | 4 +- app/services/fetch_atom_service.rb | 8 +- app/services/fetch_link_card_service.rb | 6 +- app/services/resolve_remote_account_service.rb | 3 +- app/services/send_interaction_service.rb | 14 +++- app/services/subscribe_service.rb | 48 ++++++++---- app/services/unsubscribe_service.rb | 31 +++++--- app/workers/pubsubhubbub/confirmation_worker.rb | 12 ++- app/workers/pubsubhubbub/delivery_worker.rb | 11 ++- .../concerns/signature_verification_spec.rb | 74 ++++++++++++++++++ spec/helpers/http_helper_spec.rb | 13 ---- spec/lib/request_spec.rb | 54 ++++++++++++++ .../pubsubhubbub/confirmation_worker_spec.rb | 2 +- spec/workers/pubsubhubbub/delivery_worker_spec.rb | 2 +- 23 files changed, 379 insertions(+), 90 deletions(-) create mode 100644 app/controllers/concerns/signature_verification.rb delete mode 100644 app/helpers/http_helper.rb create mode 100644 app/lib/request.rb create mode 100644 spec/controllers/concerns/signature_verification_spec.rb delete mode 100644 spec/helpers/http_helper_spec.rb create mode 100644 spec/lib/request_spec.rb (limited to 'app/services') diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 11402ab79..69b520df1 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -2,6 +2,7 @@ class AccountsController < ApplicationController include AccountControllerConcern + include SignatureVerification def show respond_to do |format| diff --git a/app/controllers/api/subscriptions_controller.rb b/app/controllers/api/subscriptions_controller.rb index d3ea98676..89007f3d6 100644 --- a/app/controllers/api/subscriptions_controller.rb +++ b/app/controllers/api/subscriptions_controller.rb @@ -42,7 +42,7 @@ class Api::SubscriptionsController < Api::BaseController end def lease_seconds_or_default - (params['hub.lease_seconds'] || 86_400).to_i.seconds + (params['hub.lease_seconds'] || 1.day).to_i.seconds end def set_account diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb new file mode 100644 index 000000000..abe845d93 --- /dev/null +++ b/app/controllers/concerns/signature_verification.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +# Implemented according to HTTP signatures (Draft 6) +# +module SignatureVerification + extend ActiveSupport::Concern + + def signed_request? + request.headers['Signature'].present? + end + + def signed_request_account + return @signed_request_account if defined?(@signed_request_account) + + unless signed_request? + @signed_request_account = nil + return + end + + raw_signature = request.headers['Signature'] + signature_params = {} + + raw_signature.split(',').each do |part| + parsed_parts = part.match(/([a-z]+)="([^"]+)"/i) + next if parsed_parts.nil? || parsed_parts.size != 3 + signature_params[parsed_parts[1]] = parsed_parts[2] + end + + if incompatible_signature?(signature_params) + @signed_request_account = nil + return + end + + account = ResolveRemoteAccountService.new.call(signature_params['keyId'].gsub(/\Aacct:/, '')) + + if account.nil? + @signed_request_account = nil + return + end + + signature = Base64.decode64(signature_params['signature']) + compare_signed_string = build_signed_string(signature_params['headers']) + + if account.keypair.public_key.verify(OpenSSL::Digest::SHA256.new, signature, compare_signed_string) + @signed_request_account = account + @signed_request_account + else + @signed_request_account = nil + end + end + + private + + def build_signed_string(signed_headers) + signed_headers = 'date' if signed_headers.blank? + + signed_headers.split(' ').map do |signed_header| + if signed_header == Request::REQUEST_TARGET + "#{Request::REQUEST_TARGET}: #{request.method.downcase} #{request.path}" + else + "#{signed_header}: #{request.headers[to_header_name(signed_header)]}" + end + end.join("\n") + end + + def matches_time_window? + begin + time_sent = DateTime.httpdate(request.headers['Date']) + rescue ArgumentError + return false + end + + (Time.now.utc - time_sent).abs <= 30 + end + + def to_header_name(name) + name.split(/-/).map(&:capitalize).join('-') + end + + def incompatible_signature?(signature_params) + signature_params['keyId'].blank? || + signature_params['signature'].blank? || + signature_params['algorithm'].blank? || + signature_params['algorithm'] != 'rsa-sha256' || + !signature_params['keyId'].start_with?('acct:') + end +end diff --git a/app/controllers/stream_entries_controller.rb b/app/controllers/stream_entries_controller.rb index 314d59619..54a435238 100644 --- a/app/controllers/stream_entries_controller.rb +++ b/app/controllers/stream_entries_controller.rb @@ -2,6 +2,7 @@ class StreamEntriesController < ApplicationController include Authorization + include SignatureVerification layout 'public' diff --git a/app/helpers/http_helper.rb b/app/helpers/http_helper.rb deleted file mode 100644 index e39a52da0..000000000 --- a/app/helpers/http_helper.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -module HttpHelper - def http_client(options = {}) - timeout = { write: 10, connect: 10, read: 10 }.merge(options) - - HTTP.headers(user_agent: user_agent) - .timeout(:per_operation, timeout) - .follow - end - - private - - def user_agent - @user_agent ||= "#{HTTP::Request::USER_AGENT} (Mastodon/#{Mastodon::Version}; +http://#{Rails.configuration.x.local_domain}/)" - end -end diff --git a/app/lib/provider_discovery.rb b/app/lib/provider_discovery.rb index 6d48cae2f..5e02e6806 100644 --- a/app/lib/provider_discovery.rb +++ b/app/lib/provider_discovery.rb @@ -1,11 +1,9 @@ # frozen_string_literal: true class ProviderDiscovery < OEmbed::ProviderDiscovery - extend HttpHelper - class << self def discover_provider(url, options = {}) - res = http_client.get(url) + res = Request.new(:get, url).perform format = options[:format] raise OEmbed::NotFound, url if res.code != 200 || res.mime_type != 'text/html' diff --git a/app/lib/request.rb b/app/lib/request.rb new file mode 100644 index 000000000..e73c5ac20 --- /dev/null +++ b/app/lib/request.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +class Request + REQUEST_TARGET = '(request-target)' + + include RoutingHelper + + def initialize(verb, url, options = {}) + @verb = verb + @url = Addressable::URI.parse(url).normalize + @options = options + @headers = {} + + set_common_headers! + end + + def on_behalf_of(account) + raise ArgumentError unless account.local? + @account = account + end + + def add_headers(new_headers) + @headers.merge!(new_headers) + end + + def perform + http_client.headers(headers).public_send(@verb, @url.to_s, @options) + end + + def headers + (@account ? @headers.merge('Signature' => signature) : @headers).without(REQUEST_TARGET) + end + + private + + def set_common_headers! + @headers[REQUEST_TARGET] = "#{@verb} #{@url.path}" + @headers['User-Agent'] = user_agent + @headers['Host'] = @url.host + @headers['Date'] = Time.now.utc.httpdate + end + + def signature + key_id = @account.to_webfinger_s + algorithm = 'rsa-sha256' + signature = Base64.strict_encode64(@account.keypair.sign(OpenSSL::Digest::SHA256.new, signed_string)) + + "keyId=\"#{key_id}\",algorithm=\"#{algorithm}\",headers=\"#{signed_headers}\",signature=\"#{signature}\"" + end + + def signed_string + @headers.map { |key, value| "#{key.downcase}: #{value}" }.join("\n") + end + + def signed_headers + @headers.keys.join(' ').downcase + end + + def user_agent + @user_agent ||= "#{HTTP::Request::USER_AGENT} (Mastodon/#{Mastodon::Version}; +#{root_url})" + end + + def timeout + { write: 10, connect: 10, read: 10 } + end + + def http_client + HTTP.timeout(:per_operation, timeout).follow + end +end diff --git a/app/models/account.rb b/app/models/account.rb index 7243cb1a5..58b0a1086 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -130,7 +130,7 @@ class Account < ApplicationRecord end def subscription(webhook_url) - OStatus2::Subscription.new(remote_url, secret: secret, lease_seconds: 86_400 * 30, webhook: webhook_url, hub: hub_url) + OStatus2::Subscription.new(remote_url, secret: secret, lease_seconds: 30.days.seconds, webhook: webhook_url, hub: hub_url) end def save_with_optional_media! diff --git a/app/models/concerns/remotable.rb b/app/models/concerns/remotable.rb index b4f169649..1bd87a642 100644 --- a/app/models/concerns/remotable.rb +++ b/app/models/concerns/remotable.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true module Remotable - include HttpHelper extend ActiveSupport::Concern included do @@ -20,7 +19,7 @@ module Remotable return if !%w(http https).include?(parsed_url.scheme) || parsed_url.host.empty? || self[attribute_name] == url begin - response = http_client.get(url) + response = Request.new(:get, url).perform return if response.code != 200 diff --git a/app/models/subscription.rb b/app/models/subscription.rb index 35a228df0..d9d5024a9 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -16,8 +16,8 @@ # class Subscription < ApplicationRecord - MIN_EXPIRATION = 7.days.seconds.to_i - MAX_EXPIRATION = 30.days.seconds.to_i + MIN_EXPIRATION = 1.day.to_i + MAX_EXPIRATION = 30.days.to_i belongs_to :account, required: true diff --git a/app/services/fetch_atom_service.rb b/app/services/fetch_atom_service.rb index d430b22e9..3ac441e3e 100644 --- a/app/services/fetch_atom_service.rb +++ b/app/services/fetch_atom_service.rb @@ -1,16 +1,14 @@ # frozen_string_literal: true class FetchAtomService < BaseService - include HttpHelper - def call(url) return if url.blank? - response = http_client.head(url) + response = Request.new(:head, url).perform Rails.logger.debug "Remote status HEAD request returned code #{response.code}" - response = http_client.get(url) if response.code == 405 + response = Request.new(:get, url).perform if response.code == 405 Rails.logger.debug "Remote status GET request returned code #{response.code}" @@ -49,6 +47,6 @@ class FetchAtomService < BaseService end def fetch(url) - http_client.get(url).to_s + Request.new(:get, url).perform.to_s end end diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb index 6ef3abb66..20c85e0ea 100644 --- a/app/services/fetch_link_card_service.rb +++ b/app/services/fetch_link_card_service.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class FetchLinkCardService < BaseService - include HttpHelper - URL_PATTERN = %r{https?://\S+} def call(status) @@ -13,7 +11,7 @@ class FetchLinkCardService < BaseService url = url.to_s card = PreviewCard.where(status: status).first_or_initialize(status: status, url: url) - res = http_client.head(url) + res = Request.new(:head, url).perform return if res.code != 200 || res.mime_type != 'text/html' @@ -80,7 +78,7 @@ class FetchLinkCardService < BaseService end def attempt_opengraph(card, url) - response = http_client.get(url) + response = Request.new(:get, url).perform return if response.code != 200 || response.mime_type != 'text/html' diff --git a/app/services/resolve_remote_account_service.rb b/app/services/resolve_remote_account_service.rb index 362d0df98..d2dfda824 100644 --- a/app/services/resolve_remote_account_service.rb +++ b/app/services/resolve_remote_account_service.rb @@ -2,7 +2,6 @@ class ResolveRemoteAccountService < BaseService include OStatus2::MagicKey - include HttpHelper DFRN_NS = 'http://purl.org/macgirvin/dfrn/1.0' @@ -79,7 +78,7 @@ class ResolveRemoteAccountService < BaseService end def get_feed(url) - response = http_client(write: 20, connect: 20, read: 50).get(Addressable::URI.parse(url).normalize) + response = Request.new(:get, url).perform raise Goldfinger::Error, "Feed attempt failed for #{url}: HTTP #{response.code}" unless response.code == 200 [response.to_s, Nokogiri::XML(response)] end diff --git a/app/services/send_interaction_service.rb b/app/services/send_interaction_service.rb index 34c8f9e34..ef38a748b 100644 --- a/app/services/send_interaction_service.rb +++ b/app/services/send_interaction_service.rb @@ -12,13 +12,23 @@ class SendInteractionService < BaseService return if block_notification? - envelope = salmon.pack(@xml, @source_account.keypair) - delivery = salmon.post(@target_account.salmon_url, envelope) + delivery = build_request.perform + raise "Delivery failed for #{target_account.salmon_url}: HTTP #{delivery.code}" unless delivery.code > 199 && delivery.code < 300 end private + def build_request + request = Request.new(:post, @target_account.salmon_url, body: envelope) + request.add_headers('Content-Type' => 'application/magic-envelope+xml') + request + end + + def envelope + salmon.pack(@xml, @source_account.keypair) + end + def block_notification? DomainBlock.blocked?(@target_account.domain) end diff --git a/app/services/subscribe_service.rb b/app/services/subscribe_service.rb index 1e7984a7f..f58067038 100644 --- a/app/services/subscribe_service.rb +++ b/app/services/subscribe_service.rb @@ -2,34 +2,54 @@ class SubscribeService < BaseService def call(account) - account.secret = SecureRandom.hex + @account = account + @account.secret = SecureRandom.hex + @response = build_request.perform - subscription = account.subscription(api_subscription_url(account.id)) - response = subscription.subscribe - - if response_failed_permanently?(response) + if response_failed_permanently? # We're not allowed to subscribe. Fail and move on. - account.secret = '' - account.save! - elsif response_successful?(response) + @account.secret = '' + @account.save! + elsif response_successful? # The subscription will be confirmed asynchronously. - account.save! + @account.save! else # The response was either a 429 rate limit, or a 5xx error. # We need to retry at a later time. Fail loudly! - raise "Subscription attempt failed for #{account.acct} (#{account.hub_url}): HTTP #{response.code}" + raise "Subscription attempt failed for #{@account.acct} (#{@account.hub_url}): HTTP #{@response.code}" end end private + def build_request + request = Request.new(:post, @account.hub_url, form: subscription_params) + request.on_behalf_of(some_local_account) if some_local_account + request + end + + def subscription_params + { + 'hub.topic': @account.remote_url, + 'hub.mode': 'subscribe', + 'hub.callback': api_subscription_url(@account.id), + 'hub.verify': 'async', + 'hub.secret': @account.secret, + 'hub.lease_seconds': 7.days.seconds, + } + end + + def some_local_account + @some_local_account ||= Account.local.first + end + # Any response in the 3xx or 4xx range, except for 429 (rate limit) - def response_failed_permanently?(response) - (response.status.redirect? || response.status.client_error?) && !response.status.too_many_requests? + def response_failed_permanently? + (@response.status.redirect? || @response.status.client_error?) && !@response.status.too_many_requests? end # Any response in the 2xx range - def response_successful?(response) - response.status.success? + def response_successful? + @response.status.success? end end diff --git a/app/services/unsubscribe_service.rb b/app/services/unsubscribe_service.rb index 6db8dbdc4..c2f022d7d 100644 --- a/app/services/unsubscribe_service.rb +++ b/app/services/unsubscribe_service.rb @@ -2,17 +2,30 @@ class UnsubscribeService < BaseService def call(account) - subscription = account.subscription(api_subscription_url(account.id)) - response = subscription.unsubscribe + @account = account + @response = build_request.perform - unless response.status.success? - Rails.logger.debug "PuSH unsubscribe for #{account.acct} failed: #{response.status}" - end + Rails.logger.debug "PuSH unsubscribe for #{@account.acct} failed: #{@response.status}" unless @response.status.success? - account.secret = '' - account.subscription_expires_at = nil - account.save! + @account.secret = '' + @account.subscription_expires_at = nil + @account.save! rescue HTTP::Error, OpenSSL::SSL::SSLError - Rails.logger.debug "PuSH subscription request for #{account.acct} could not be made due to HTTP or SSL error" + Rails.logger.debug "PuSH subscription request for #{@account.acct} could not be made due to HTTP or SSL error" + end + + private + + def build_request + Request.new(:post, @account.hub_url, form: subscription_params) + end + + def subscription_params + { + 'hub.topic': @account.remote_url, + 'hub.mode': 'unsubscribe', + 'hub.callback': api_subscription_url(@account.id), + 'hub.verify': 'async', + } end end diff --git a/app/workers/pubsubhubbub/confirmation_worker.rb b/app/workers/pubsubhubbub/confirmation_worker.rb index 9186c5d7d..e1ccfb99c 100644 --- a/app/workers/pubsubhubbub/confirmation_worker.rb +++ b/app/workers/pubsubhubbub/confirmation_worker.rb @@ -60,9 +60,7 @@ class Pubsubhubbub::ConfirmationWorker end def callback_get_with_params - HTTP.headers(user_agent: 'Mastodon/PubSubHubbub') - .timeout(:per_operation, write: 20, connect: 20, read: 50) - .get(subscription.callback_url, params: callback_params) + Request.new(:get, subscription.callback_url, params: callback_params).perform end def callback_response_body @@ -71,10 +69,10 @@ class Pubsubhubbub::ConfirmationWorker def callback_params { - 'hub.topic' => account_url(subscription.account, format: :atom), - 'hub.mode' => mode, - 'hub.challenge' => challenge, - 'hub.lease_seconds' => subscription.lease_seconds, + 'hub.topic': account_url(subscription.account, format: :atom), + 'hub.mode': mode, + 'hub.challenge': challenge, + 'hub.lease_seconds': subscription.lease_seconds, } end diff --git a/app/workers/pubsubhubbub/delivery_worker.rb b/app/workers/pubsubhubbub/delivery_worker.rb index 981838f33..05d160cf7 100644 --- a/app/workers/pubsubhubbub/delivery_worker.rb +++ b/app/workers/pubsubhubbub/delivery_worker.rb @@ -33,9 +33,9 @@ class Pubsubhubbub::DeliveryWorker end def callback_post_payload - HTTP.timeout(:per_operation, write: 50, connect: 20, read: 50) - .headers(headers) - .post(subscription.callback_url, body: payload) + request = Request.new(:post, subscription.callback_url, body: payload) + request.add_headers(headers) + request.perform end def blocked_domain? @@ -48,13 +48,12 @@ class Pubsubhubbub::DeliveryWorker def headers { - 'User-Agent' => 'Mastodon/PubSubHubbub', 'Content-Type' => 'application/atom+xml', - 'Link' => link_headers, + 'Link' => link_header, }.merge(signature_headers.to_h) end - def link_headers + def link_header LinkHeader.new([hub_link_header, self_link_header]).to_s end diff --git a/spec/controllers/concerns/signature_verification_spec.rb b/spec/controllers/concerns/signature_verification_spec.rb new file mode 100644 index 000000000..b371795ab --- /dev/null +++ b/spec/controllers/concerns/signature_verification_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ApplicationController, type: :controller do + controller do + include SignatureVerification + + def success + head 200 + end + + def alternative_success + head 200 + end + end + + before do + routes.draw { get 'success' => 'anonymous#success' } + end + + context 'without signature header' do + before do + get :success + end + + describe '#signed_request?' do + it 'returns false' do + expect(controller.signed_request?).to be false + end + end + + describe '#signed_request_account' do + it 'returns nil' do + expect(controller.signed_request_account).to be_nil + end + end + end + + context 'with signature header' do + let!(:author) { Fabricate(:account) } + + before do + get :success + + fake_request = Request.new(:get, request.url) + fake_request.on_behalf_of(author) + + request.headers.merge!(fake_request.headers) + end + + describe '#signed_request?' do + it 'returns true' do + expect(controller.signed_request?).to be true + end + end + + describe '#signed_request_account' do + it 'returns an account' do + expect(controller.signed_request_account).to eq author + end + + it 'returns nil when path does not match' do + request.path = '/alternative-path' + expect(controller.signed_request_account).to be_nil + end + + it 'returns nil when method does not match' do + post :success + expect(controller.signed_request_account).to be_nil + end + end + end +end diff --git a/spec/helpers/http_helper_spec.rb b/spec/helpers/http_helper_spec.rb deleted file mode 100644 index b8e31b8e6..000000000 --- a/spec/helpers/http_helper_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe HttpHelper do - describe 'http_client' do - it 'returns HTTP::Client with default options' do - options = helper.http_client.default_options - expect(options.headers['User-Agent']).to match /.+ \(Mastodon\/.+;\ \+http:\/\/cb6e6126\.ngrok\.io\/\)/ - expect(options.timeout_options).to eq read_timeout: 10, write_timeout: 10, connect_timeout: 10 - end - end -end diff --git a/spec/lib/request_spec.rb b/spec/lib/request_spec.rb new file mode 100644 index 000000000..782f14b18 --- /dev/null +++ b/spec/lib/request_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Request do + subject { Request.new(:get, 'http://example.com') } + + describe '#headers' do + it 'returns user agent' do + expect(subject.headers['User-Agent']).to be_present + end + + it 'returns the date header' do + expect(subject.headers['Date']).to be_present + end + + it 'returns the host header' do + expect(subject.headers['Host']).to be_present + end + + it 'does not return virtual request-target header' do + expect(subject.headers['(request-target)']).to be_nil + end + end + + describe '#on_behalf_of' do + it 'when used, adds signature header' do + subject.on_behalf_of(Fabricate(:account)) + expect(subject.headers['Signature']).to be_present + end + end + + describe '#add_headers' do + it 'adds headers to the request' do + subject.add_headers('Test' => 'Foo') + expect(subject.headers['Test']).to eq 'Foo' + end + end + + describe '#perform' do + before do + stub_request(:get, 'http://example.com') + subject.perform + end + + it 'executes a HTTP request' do + expect(a_request(:get, 'http://example.com')).to have_been_made.once + end + + it 'sets headers' do + expect(a_request(:get, 'http://example.com').with(headers: subject.headers)).to have_been_made + end + end +end diff --git a/spec/workers/pubsubhubbub/confirmation_worker_spec.rb b/spec/workers/pubsubhubbub/confirmation_worker_spec.rb index 1199d5801..8f66b4520 100644 --- a/spec/workers/pubsubhubbub/confirmation_worker_spec.rb +++ b/spec/workers/pubsubhubbub/confirmation_worker_spec.rb @@ -83,6 +83,6 @@ describe Pubsubhubbub::ConfirmationWorker do end def http_headers - { 'Connection' => 'close', 'Host' => 'example.com', 'User-Agent' => 'Mastodon/PubSubHubbub' } + { 'Connection' => 'close', 'Host' => 'example.com', 'User-Agent' => 'http.rb/2.2.2 (Mastodon/1.4.7; +https://cb6e6126.ngrok.io/)' } end end diff --git a/spec/workers/pubsubhubbub/delivery_worker_spec.rb b/spec/workers/pubsubhubbub/delivery_worker_spec.rb index 081dfa41c..a83245786 100644 --- a/spec/workers/pubsubhubbub/delivery_worker_spec.rb +++ b/spec/workers/pubsubhubbub/delivery_worker_spec.rb @@ -59,7 +59,7 @@ describe Pubsubhubbub::DeliveryWorker do 'Content-Type' => 'application/atom+xml', 'Host' => 'example.com', 'Link' => "; rel=\"hub\", ; rel=\"self\"", - 'User-Agent' => 'Mastodon/PubSubHubbub', + 'User-Agent' => 'http.rb/2.2.2 (Mastodon/1.4.7; +https://cb6e6126.ngrok.io/)', }.tap do |basic| known_digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), subscription.secret.to_s, payload) basic.merge('X-Hub-Signature' => "sha1=#{known_digest}") if subscription.secret? -- cgit From cd9b2ab2f70b6c1da5d0abeaa88eecdfc1b41f78 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 14 Jul 2017 23:01:20 +0200 Subject: Fix #2672 - Connect signed PuSH subscription requests to instance domain (#4205) * Fix #2672 - Connect signed PuSH subscription requests to instance domain Resolves #2739 * Fix return of locate_subscription * Fix tests --- app/controllers/api/push_controller.rb | 8 +++++++- app/models/subscription.rb | 2 +- app/services/pubsubhubbub/subscribe_service.rb | 16 +++++++++++++--- app/workers/pubsubhubbub/distribution_worker.rb | 8 ++++---- db/migrate/20170714184731_add_domain_to_subscriptions.rb | 5 +++++ db/schema.rb | 3 ++- spec/controllers/api/push_controller_spec.rb | 1 + 7 files changed, 33 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20170714184731_add_domain_to_subscriptions.rb (limited to 'app/services') diff --git a/app/controllers/api/push_controller.rb b/app/controllers/api/push_controller.rb index 951867140..e04d19125 100644 --- a/app/controllers/api/push_controller.rb +++ b/app/controllers/api/push_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Api::PushController < Api::BaseController + include SignatureVerification + def update response, status = process_push_request render plain: response, status: status @@ -11,7 +13,7 @@ class Api::PushController < Api::BaseController def process_push_request case hub_mode when 'subscribe' - Pubsubhubbub::SubscribeService.new.call(account_from_topic, hub_callback, hub_secret, hub_lease_seconds) + Pubsubhubbub::SubscribeService.new.call(account_from_topic, hub_callback, hub_secret, hub_lease_seconds, verified_domain) when 'unsubscribe' Pubsubhubbub::UnsubscribeService.new.call(account_from_topic, hub_callback) else @@ -57,6 +59,10 @@ class Api::PushController < Api::BaseController TagManager.instance.web_domain?(hub_topic_domain) end + def verified_domain + return signed_request_account.domain if signed_request_account + end + def hub_topic_domain hub_topic_uri.host + (hub_topic_uri.port ? ":#{hub_topic_uri.port}" : '') end diff --git a/app/models/subscription.rb b/app/models/subscription.rb index d9d5024a9..bf643c1f9 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true - # == Schema Information # # Table name: subscriptions @@ -13,6 +12,7 @@ # created_at :datetime not null # updated_at :datetime not null # last_successful_delivery_at :datetime +# domain :string # class Subscription < ApplicationRecord diff --git a/app/services/pubsubhubbub/subscribe_service.rb b/app/services/pubsubhubbub/subscribe_service.rb index eeb7ab258..2dba05b12 100644 --- a/app/services/pubsubhubbub/subscribe_service.rb +++ b/app/services/pubsubhubbub/subscribe_service.rb @@ -3,13 +3,15 @@ class Pubsubhubbub::SubscribeService < BaseService URL_PATTERN = /\A#{URI.regexp(%w(http https))}\z/ - attr_reader :account, :callback, :secret, :lease_seconds + attr_reader :account, :callback, :secret, + :lease_seconds, :domain - def call(account, callback, secret, lease_seconds) + def call(account, callback, secret, lease_seconds, verified_domain = nil) @account = account @callback = Addressable::URI.parse(callback).normalize.to_s @secret = secret @lease_seconds = lease_seconds + @domain = verified_domain process_subscribe end @@ -56,6 +58,14 @@ class Pubsubhubbub::SubscribeService < BaseService end def locate_subscription - Subscription.where(account: account, callback_url: callback).first_or_create!(account: account, callback_url: callback) + subscription = Subscription.find_by(account: account, callback_url: callback) + + if subscription.nil? + subscription = Subscription.new(account: account, callback_url: callback) + end + + subscription.domain = domain + subscription.save! + subscription end end diff --git a/app/workers/pubsubhubbub/distribution_worker.rb b/app/workers/pubsubhubbub/distribution_worker.rb index b41cec90d..7592354cc 100644 --- a/app/workers/pubsubhubbub/distribution_worker.rb +++ b/app/workers/pubsubhubbub/distribution_worker.rb @@ -35,16 +35,16 @@ class Pubsubhubbub::DistributionWorker @payload = AtomSerializer.render(AtomSerializer.new.feed(@account, stream_entries)) @domains = @account.followers.domains - Pubsubhubbub::DeliveryWorker.push_bulk(@subscriptions.reject { |s| !allowed_to_receive?(s.callback_url) }) do |subscription| + Pubsubhubbub::DeliveryWorker.push_bulk(@subscriptions.reject { |s| !allowed_to_receive?(s.callback_url, s.domain) }) do |subscription| [subscription.id, @payload] end end def active_subscriptions - Subscription.where(account: @account).active.select('id, callback_url') + Subscription.where(account: @account).active.select('id, callback_url, domain') end - def allowed_to_receive?(callback_url) - @domains.include?(Addressable::URI.parse(callback_url).host) + def allowed_to_receive?(callback_url, domain) + (!domain.nil? && @domains.include?(domain)) || @domains.include?(Addressable::URI.parse(callback_url).host) end end diff --git a/db/migrate/20170714184731_add_domain_to_subscriptions.rb b/db/migrate/20170714184731_add_domain_to_subscriptions.rb new file mode 100644 index 000000000..7c01a64f5 --- /dev/null +++ b/db/migrate/20170714184731_add_domain_to_subscriptions.rb @@ -0,0 +1,5 @@ +class AddDomainToSubscriptions < ActiveRecord::Migration[5.1] + def change + add_column :subscriptions, :domain, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index b2c59a0f6..5ec78a7c9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170713190709) do +ActiveRecord::Schema.define(version: 20170714184731) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -326,6 +326,7 @@ ActiveRecord::Schema.define(version: 20170713190709) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.datetime "last_successful_delivery_at" + t.string "domain" t.index ["account_id", "callback_url"], name: "index_subscriptions_on_account_id_and_callback_url", unique: true end diff --git a/spec/controllers/api/push_controller_spec.rb b/spec/controllers/api/push_controller_spec.rb index 18bfa70e5..647698bd1 100644 --- a/spec/controllers/api/push_controller_spec.rb +++ b/spec/controllers/api/push_controller_spec.rb @@ -21,6 +21,7 @@ RSpec.describe Api::PushController, type: :controller do 'https://callback.host/api', 'as1234df', '3600', + nil ) expect(response).to have_http_status(:success) end -- cgit From 05cd37097c134d559550adca6cd17cf7578be94b Mon Sep 17 00:00:00 2001 From: ThibG Date: Sat, 15 Jul 2017 17:24:35 +0200 Subject: Optimize uri normalization (#4212) * Add dependency on idn-ruby to speed up URI normalization * Use normalized_host instead of normalize.host when applicable When we are only interested in the normalized host, calling normalized_host avoids normalizing the other components of the URI as well as creating a new object --- Gemfile | 1 + Gemfile.lock | 2 ++ app/lib/tag_manager.rb | 2 +- app/services/concerns/author_extractor.rb | 2 +- app/services/fetch_remote_status_service.rb | 4 ++-- app/workers/pubsubhubbub/delivery_worker.rb | 2 +- 6 files changed, 8 insertions(+), 5 deletions(-) (limited to 'app/services') diff --git a/Gemfile b/Gemfile index 5d5ddfae1..a6c2b2d65 100644 --- a/Gemfile +++ b/Gemfile @@ -36,6 +36,7 @@ gem 'htmlentities', '~> 4.3' gem 'http', '~> 2.2' gem 'http_accept_language', '~> 2.1' gem 'httplog', '~> 0.99' +gem 'idn-ruby', require: 'idn' gem 'kaminari', '~> 1.0' gem 'link_header', '~> 0.0' gem 'mime-types', '~> 3.1' diff --git a/Gemfile.lock b/Gemfile.lock index daef3e1ad..f637c9bbe 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -208,6 +208,7 @@ GEM parser (>= 2.2.3.0) rainbow (~> 2.2) terminal-table (>= 1.5.1) + idn-ruby (0.1.0) jmespath (1.3.1) json (2.1.0) jsonapi-renderer (0.1.2) @@ -528,6 +529,7 @@ DEPENDENCIES http_accept_language (~> 2.1) httplog (~> 0.99) i18n-tasks (~> 0.9) + idn-ruby kaminari (~> 1.0) letter_opener (~> 1.4) letter_opener_web (~> 1.3) diff --git a/app/lib/tag_manager.rb b/app/lib/tag_manager.rb index f1a2234dc..5f87a2a48 100644 --- a/app/lib/tag_manager.rb +++ b/app/lib/tag_manager.rb @@ -70,7 +70,7 @@ class TagManager uri = Addressable::URI.new uri.host = domain.gsub(/[\/]/, '') - uri.normalize.host + uri.normalized_host end def same_acct?(canonical, needle) diff --git a/app/services/concerns/author_extractor.rb b/app/services/concerns/author_extractor.rb index 00fe1c663..867d6dc25 100644 --- a/app/services/concerns/author_extractor.rb +++ b/app/services/concerns/author_extractor.rb @@ -14,7 +14,7 @@ module AuthorExtractor return nil if username.blank? || uri.blank? - domain = Addressable::URI.parse(uri).normalize.host + domain = Addressable::URI.parse(uri).normalized_host acct = "#{username}@#{domain}" end diff --git a/app/services/fetch_remote_status_service.rb b/app/services/fetch_remote_status_service.rb index 4cfd33d90..6ac31e4d8 100644 --- a/app/services/fetch_remote_status_service.rb +++ b/app/services/fetch_remote_status_service.rb @@ -24,7 +24,7 @@ class FetchRemoteStatusService < BaseService xml.encoding = 'utf-8' account = author_from_xml(xml.at_xpath('/xmlns:entry', xmlns: TagManager::XMLNS)) - domain = Addressable::URI.parse(url).normalize.host + domain = Addressable::URI.parse(url).normalized_host return nil unless !account.nil? && confirmed_domain?(domain, account) @@ -39,6 +39,6 @@ class FetchRemoteStatusService < BaseService end def confirmed_domain?(domain, account) - account.domain.nil? || domain.casecmp(account.domain).zero? || domain.casecmp(Addressable::URI.parse(account.remote_url).normalize.host).zero? + account.domain.nil? || domain.casecmp(account.domain).zero? || domain.casecmp(Addressable::URI.parse(account.remote_url).normalized_host).zero? end end diff --git a/app/workers/pubsubhubbub/delivery_worker.rb b/app/workers/pubsubhubbub/delivery_worker.rb index 05d160cf7..2e1101b93 100644 --- a/app/workers/pubsubhubbub/delivery_worker.rb +++ b/app/workers/pubsubhubbub/delivery_worker.rb @@ -43,7 +43,7 @@ class Pubsubhubbub::DeliveryWorker end def host - Addressable::URI.parse(subscription.callback_url).normalize.host + Addressable::URI.parse(subscription.callback_url).normalized_host end def headers -- cgit