From 7a0232249afee843f6fa1375f269df69a54618db Mon Sep 17 00:00:00 2001 From: Kyle Zhao Date: Mon, 17 Sep 2018 16:31:46 +0800 Subject: [PATCH 01/56] extract inline JS that's used to store preloaded data (#6370) --- .../javascripts/preload-application-data.js | 12 ++++++++++++ app/helpers/application_helper.rb | 7 ++++++- app/views/layouts/application.html.erb | 14 +++----------- config/application.rb | 1 + spec/helpers/application_helper_spec.rb | 11 +++++++++++ spec/requests/categories_controller_spec.rb | 6 +++++- spec/requests/users_controller_spec.rb | 19 ++++++++++++++----- 7 files changed, 52 insertions(+), 18 deletions(-) create mode 100644 app/assets/javascripts/preload-application-data.js diff --git a/app/assets/javascripts/preload-application-data.js b/app/assets/javascripts/preload-application-data.js new file mode 100644 index 0000000000..58b138f670 --- /dev/null +++ b/app/assets/javascripts/preload-application-data.js @@ -0,0 +1,12 @@ +(function() { + var preloadedDataElement = document.getElementById("data-preloaded"); + + if (preloadedDataElement) { + var ps = require("preload-store").default; + var preloaded = JSON.parse(preloadedDataElement.dataset.preloaded); + + Object.keys(preloaded).forEach(function(key) { + ps.store(key, JSON.parse(preloaded[key])); + }); + } +})(); diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f775b6e842..071e6bf250 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -129,7 +129,7 @@ module ApplicationHelper javascript = javascript.scrub javascript.gsub!(/\342\200\250/u, '
') javascript.gsub!(/(<\/)/u, '\u003C/') - javascript.html_safe + javascript else '' end @@ -401,4 +401,9 @@ module ApplicationHelper Stylesheet::Manager.stylesheet_link_tag(name, 'all', ids) end + + def preloaded_json + return '{}' if @preloaded.blank? + @preloaded.transform_values { |value| escape_unicode(value) }.to_json + end end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index b815383d67..9dc4e992a3 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -55,6 +55,9 @@ <%= render partial: "common/discourse_stylesheet" %> <%= render partial: "common/special_font_face" %> + + <%= preload_script "preload-application-data" %> + <%= yield :head %> <%= build_plugin_html 'server:before-head-close' %> @@ -104,17 +107,6 @@ <% end %> - <%- if @preloaded.present? %> - - <%- end %> - <%= yield :data %> <%= render :partial => "common/discourse_javascript" %> diff --git a/config/application.rb b/config/application.rb index 8687840329..9c23554c14 100644 --- a/config/application.rb +++ b/config/application.rb @@ -119,6 +119,7 @@ module Discourse service-worker.js google-tag-manager.js google-universal-analytics.js + preload-application-data.js } # Precompile all available locales diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 06553a4fde..f838326f97 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -165,4 +165,15 @@ describe ApplicationHelper do end end + describe 'preloaded_json' do + it 'returns empty JSON if preloaded is empty' do + @preloaded = nil + expect(helper.preloaded_json).to eq('{}') + end + + it 'escapes and strips invalid unicode and strips in json body' do + @preloaded = { test: %{["< \x80"]} } + expect(helper.preloaded_json).to eq(%{{"test":"[\\"\\u003c \uFFFD\\"]"}}) + end + end end diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index fa6145d649..d6aba32f3a 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -31,7 +31,11 @@ describe CategoriesController do SiteSetting.categories_topics = 5 SiteSetting.categories_topics.times { Fabricate(:topic) } get "/categories" - expect(response.body).to include(%{"more_topics_url":"/latest"}) + + expect(response.body).to have_tag("meta#data-preloaded") do |element| + json = JSON.parse(element.current_scope.attribute('data-preloaded').value) + expect(json['topic_list_latest']).to include(%{"more_topics_url":"/latest"}) + end end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 717b8ccdd7..ef1ac79355 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -190,7 +190,10 @@ describe UsersController do ) expect(response.status).to eq(200) - expect(response.body).to include('{"is_developer":false,"admin":false,"second_factor_required":false,"backup_enabled":false}') + expect(response.body).to have_tag("meta#data-preloaded") do |element| + json = JSON.parse(element.current_scope.attribute('data-preloaded').value) + expect(json['password_reset']).to include('{"is_developer":false,"admin":false,"second_factor_required":false,"backup_enabled":false}') + end expect(session["password-#{token}"]).to be_blank expect(UserAuthToken.where(id: user_auth_token.id).count).to eq(0) @@ -255,7 +258,10 @@ describe UsersController do get "/u/password-reset/#{token}" - expect(response.body).to include('{"is_developer":false,"admin":false,"second_factor_required":true,"backup_enabled":false}') + expect(response.body).to have_tag("meta#data-preloaded") do |element| + json = JSON.parse(element.current_scope.attribute('data-preloaded').value) + expect(json['password_reset']).to include('{"is_developer":false,"admin":false,"second_factor_required":true,"backup_enabled":false}') + end put "/u/password-reset/#{token}", params: { password: 'hg9ow8yHG32O', @@ -2592,9 +2598,12 @@ describe UsersController do expect(response.status).to eq(200) - expect(response.body).to include( - "{\"message\":\"#{I18n.t("login.activate_email", email: user.email).gsub!(" Date: Mon, 17 Sep 2018 10:36:14 +0200 Subject: [PATCH 02/56] FIX: ensures onSelect/onDeselect are called --- .../components/single-select.js.es6 | 4 ++ .../components/single-select-test.js.es6 | 52 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/app/assets/javascripts/select-kit/components/single-select.js.es6 b/app/assets/javascripts/select-kit/components/single-select.js.es6 index 7713d51194..af3aa3a4f8 100644 --- a/app/assets/javascripts/select-kit/components/single-select.js.es6 +++ b/app/assets/javascripts/select-kit/components/single-select.js.es6 @@ -210,6 +210,10 @@ export default SelectKitComponent.extend({ }, select(computedContentItem) { + if (this.get("hasSelection")) { + this.deselect(this.get("selection.value")); + } + if ( !computedContentItem || computedContentItem.__sk_row_type === "noneRow" diff --git a/test/javascripts/components/single-select-test.js.es6 b/test/javascripts/components/single-select-test.js.es6 index 439781e202..72366fdf72 100644 --- a/test/javascripts/components/single-select-test.js.es6 +++ b/test/javascripts/components/single-select-test.js.es6 @@ -835,3 +835,55 @@ componentTest("without forceEscape", { ); } }); + +componentTest("onSelect", { + template: + "
{{single-select content=content onSelect=(action externalAction)}}", + + beforeEach() { + this.set("externalAction", actual => { + find(".test-external-action").text(actual); + }); + + this.set("content", ["red", "blue"]); + }, + + async test(assert) { + await this.get("subject").expand(); + await this.get("subject").selectRowByValue("red"); + + assert.equal( + find(".test-external-action") + .text() + .trim(), + "red" + ); + } +}); + +componentTest("onDeselect", { + template: + "
{{single-select content=content onDeselect=(action externalAction)}}", + + beforeEach() { + this.set("externalAction", actual => { + find(".test-external-action").text(actual); + }); + + this.set("content", ["red", "blue"]); + }, + + async test(assert) { + await this.get("subject").expand(); + await this.get("subject").selectRowByValue("red"); + await this.get("subject").expand(); + await this.get("subject").selectRowByValue("blue"); + + assert.equal( + find(".test-external-action") + .text() + .trim(), + "red" + ); + } +}); From 4481836de2feb4813b6282a6ec4ae4fdde509627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 17 Sep 2018 10:31:15 +0200 Subject: [PATCH 03/56] FEATURE: new 'search_ignore_accents' site setting --- app/services/search_indexer.rb | 23 ++++++++++----------- config/locales/server.en.yml | 1 + config/site_settings.yml | 17 +++++++++++++++- lib/search/grouped_search_results.rb | 5 +++-- spec/services/search_indexer_spec.rb | 30 ++++++++++++++++------------ 5 files changed, 48 insertions(+), 28 deletions(-) diff --git a/app/services/search_indexer.rb b/app/services/search_indexer.rb index f76b9037e8..7224087368 100644 --- a/app/services/search_indexer.rb +++ b/app/services/search_indexer.rb @@ -11,8 +11,8 @@ class SearchIndexer @disabled = false end - def self.scrub_html_for_search(html) - HtmlScrubber.scrub(html) + def self.scrub_html_for_search(html, strip_diacritics: SiteSetting.search_ignore_accents) + HtmlScrubber.scrub(html, strip_diacritics: strip_diacritics) end def self.inject_extra_terms(raw) @@ -169,18 +169,10 @@ class SearchIndexer DIACRITICS ||= /([\u0300-\u036f]|[\u1AB0-\u1AFF]|[\u1DC0-\u1DFF]|[\u20D0-\u20FF])/ - def self.strip_diacritics(str) - s = str.unicode_normalize(:nfkd) - s.gsub!(DIACRITICS, "") - s.strip! - s - end - attr_reader :scrubbed def initialize(strip_diacritics: false) @scrubbed = +"" - # for now we are disabling this per: https://meta.discourse.org/t/discourse-should-ignore-if-a-character-is-accented-when-doing-a-search/90198/16?u=sam @strip_diacritics = strip_diacritics end @@ -189,7 +181,7 @@ class SearchIndexer me = new(strip_diacritics: strip_diacritics) Nokogiri::HTML::SAX::Parser.new(me).parse("
#{html}
") - me.scrubbed + me.scrubbed.squish end ATTRIBUTES ||= %w{alt title href data-youtube-title} @@ -204,8 +196,15 @@ class SearchIndexer end end + def strip_diacritics(str) + s = str.unicode_normalize(:nfkd) + s.gsub!(DIACRITICS, "") + s.strip! + s + end + def characters(str) - str = HtmlScrubber.strip_diacritics(str) if @strip_diacritics + str = strip_diacritics(str) if @strip_diacritics scrubbed << " #{str} " end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2a26a8c303..cf2f6ef211 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1144,6 +1144,7 @@ en: log_search_queries: "Log search queries performed by users" search_query_log_max_size: "Maximum amount of search queries to keep" search_query_log_max_retention_days: "Maximum amount of time to keep search queries, in days." + search_ignore_accents: "Ignore accents when searching for text." allow_uncategorized_topics: "Allow topics to be created without a category. WARNING: If there are any uncategorized topics, you must recategorize them before turning this off." allow_duplicate_topic_titles: "Allow topics with identical, duplicate titles." unique_posts_mins: "How many minutes before a user can make a post with the same content again" diff --git a/config/site_settings.yml b/config/site_settings.yml index 91d17c9730..e37054a60b 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1431,7 +1431,6 @@ search: zh_TW: 2 ko: 2 ja: 2 - search_tokenize_chinese_japanese_korean: false search_prefer_recent_posts: false search_recent_posts_size: @@ -1446,6 +1445,22 @@ search: search_query_log_max_retention_days: default: 365 # 1 year max: 1825 # 5 years + search_ignore_accents: + default: false + locale_default: + ar: true + ca: true + cs: true + el: true + es: true + fa_IR: true + fr: true + hu: true + pt: true + pt_BR: true + ro: true + sk: true + tr_TR: true uncategorized: version_checks: diff --git a/lib/search/grouped_search_results.rb b/lib/search/grouped_search_results.rb index 04ad93fa13..1f81e43e2c 100644 --- a/lib/search/grouped_search_results.rb +++ b/lib/search/grouped_search_results.rb @@ -63,13 +63,14 @@ class Search end def self.blurb_for(cooked, term = nil, blurb_length = 200) - cooked = SearchIndexer::HtmlScrubber.scrub(cooked).squish - blurb = nil + cooked = SearchIndexer.scrub_html_for_search(cooked) + if term terms = term.split(/\s+/) blurb = TextHelper.excerpt(cooked, terms.first, radius: blurb_length / 2, seperator: " ") end + blurb = TextHelper.truncate(cooked, length: blurb_length, seperator: " ") if blurb.blank? Sanitize.clean(blurb) end diff --git a/spec/services/search_indexer_spec.rb b/spec/services/search_indexer_spec.rb index a9f309a489..3886eb025a 100644 --- a/spec/services/search_indexer_spec.rb +++ b/spec/services/search_indexer_spec.rb @@ -3,6 +3,10 @@ require 'rails_helper' describe SearchIndexer do let(:post_id) { 99 } + def scrub(html, strip_diacritics: false) + SearchIndexer.scrub_html_for_search(html, strip_diacritics: strip_diacritics) + end + it 'correctly indexes chinese' do SiteSetting.default_locale = 'zh_CN' data = "你好世界" @@ -16,26 +20,26 @@ describe SearchIndexer do it 'extract youtube title' do html = "
" - - scrubbed = SearchIndexer::HtmlScrubber.scrub(html) - - expect(scrubbed).to eq(" Metallica Mixer Explains Missing Bass on 'And Justice for All' [Exclusive] ") + scrubbed = scrub(html) + expect(scrubbed).to eq("Metallica Mixer Explains Missing Bass on 'And Justice for All' [Exclusive]") end it 'extract a link' do html = "link" - - scrubbed = SearchIndexer::HtmlScrubber.scrub(html) - - expect(scrubbed).to eq(" http://meta.discourse.org/ link ") + scrubbed = scrub(html) + expect(scrubbed).to eq("http://meta.discourse.org/ link") end - it 'removes diacritics' do + it 'uses ignore_accent setting to strip diacritics' do html = "

HELLO Hétérogénéité Здравствуйте هتاف للترحيب 你好

" - scrubbed = SearchIndexer::HtmlScrubber.scrub(html, strip_diacritics: true) + SiteSetting.search_ignore_accents = true + scrubbed = SearchIndexer.scrub_html_for_search(html) + expect(scrubbed).to eq("HELLO Heterogeneite Здравствуите هتاف للترحيب 你好") - expect(scrubbed).to eq(" HELLO Heterogeneite Здравствуите هتاف للترحيب 你好 ") + SiteSetting.search_ignore_accents = false + scrubbed = SearchIndexer.scrub_html_for_search(html) + expect(scrubbed).to eq("HELLO Hétérogénéité Здравствуйте هتاف للترحيب 你好") end it "doesn't index local files" do @@ -54,9 +58,9 @@ describe SearchIndexer do HTML - scrubbed = SearchIndexer::HtmlScrubber.scrub(html).gsub(/\s+/, " ") + scrubbed = scrub(html) - expect(scrubbed).to eq(" Discourse 51%20PM Untitled design (21).jpg Untitled%20design%20(21) Untitled design (21).jpg 1280x1136 472 KB ") + expect(scrubbed).to eq("Discourse 51%20PM Untitled design (21).jpg Untitled%20design%20(21) Untitled design (21).jpg 1280x1136 472 KB") end it 'correctly indexes a post according to version' do From b13b6e30d62c518f7a158da9df12a2a3d6528682 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Mon, 17 Sep 2018 18:18:43 +0530 Subject: [PATCH 04/56] DEV: Skip converting local dates as json --- plugins/discourse-local-dates/plugin.rb | 2 +- .../spec/models/post_spec.rb | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 plugins/discourse-local-dates/spec/models/post_spec.rb diff --git a/plugins/discourse-local-dates/plugin.rb b/plugins/discourse-local-dates/plugin.rb index cb243194d7..bb4daab5af 100644 --- a/plugins/discourse-local-dates/plugin.rb +++ b/plugins/discourse-local-dates/plugin.rb @@ -37,7 +37,7 @@ after_initialize do end if dates.present? - post.custom_fields[DiscourseLocalDates::POST_CUSTOM_FIELD] = dates.to_json + post.custom_fields[DiscourseLocalDates::POST_CUSTOM_FIELD] = dates post.save_custom_fields elsif !post.custom_fields[DiscourseLocalDates::POST_CUSTOM_FIELD].nil? post.custom_fields.delete(DiscourseLocalDates::POST_CUSTOM_FIELD) diff --git a/plugins/discourse-local-dates/spec/models/post_spec.rb b/plugins/discourse-local-dates/spec/models/post_spec.rb new file mode 100644 index 0000000000..15da7f75fd --- /dev/null +++ b/plugins/discourse-local-dates/spec/models/post_spec.rb @@ -0,0 +1,26 @@ +require 'rails_helper' + +describe Post do + + before do + SiteSetting.queue_jobs = false + end + + describe '#local_dates' do + it "should have correct custom fields" do + post = Fabricate(:post, raw: <<~SQL) + [date=2018-09-17 time=01:39:00 format="LLL" timezones="Europe/Paris|America/Los_Angeles"] + SQL + CookedPostProcessor.new(post).post_process + + expect(post.local_dates).to eq([{"date"=>"2018-09-17", "time"=>"01:39:00"}]) + + post.raw = "Text removed" + post.save + CookedPostProcessor.new(post).post_process + + expect(post.local_dates).to eq([]) + end + end + +end From 16d7132ba19430c15407fc603e78600ccba6aa6a Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Mon, 17 Sep 2018 18:39:59 +0530 Subject: [PATCH 05/56] SPEC: Check date and time values separately --- plugins/discourse-local-dates/spec/models/post_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/discourse-local-dates/spec/models/post_spec.rb b/plugins/discourse-local-dates/spec/models/post_spec.rb index 15da7f75fd..29793fb541 100644 --- a/plugins/discourse-local-dates/spec/models/post_spec.rb +++ b/plugins/discourse-local-dates/spec/models/post_spec.rb @@ -13,7 +13,9 @@ describe Post do SQL CookedPostProcessor.new(post).post_process - expect(post.local_dates).to eq([{"date"=>"2018-09-17", "time"=>"01:39:00"}]) + expect(post.local_dates.count).to eq(1) + expect(post.local_dates[0]["date"]).to eq("2018-09-17") + expect(post.local_dates[0]["time"]).to eq("01:39:00") post.raw = "Text removed" post.save From b9891c26417df1777b0cfc89ec41466436e10801 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Mon, 17 Sep 2018 09:57:11 -0400 Subject: [PATCH 06/56] FIX: error because last_id is nil in discourse_merger script --- script/bulk_import/discourse_merger.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/script/bulk_import/discourse_merger.rb b/script/bulk_import/discourse_merger.rb index e3d4a752b0..14cf5a1fdf 100644 --- a/script/bulk_import/discourse_merger.rb +++ b/script/bulk_import/discourse_merger.rb @@ -173,7 +173,7 @@ class BulkImport::DiscourseMerger < BulkImport::Base columns = Category.columns.map(&:name) imported_ids = [] - last_id = Category.unscoped.maximum(:id) + last_id = Category.unscoped.maximum(:id) || 1 sql = "COPY categories (#{columns.map { |c| "\"#{c}\"" }.join(', ')}) FROM STDIN" @raw_connection.copy_data(sql, @encoder) do @@ -249,7 +249,7 @@ class BulkImport::DiscourseMerger < BulkImport::Base columns = Tag.columns.map(&:name) imported_ids = [] - last_id = Tag.unscoped.maximum(:id) + last_id = Tag.unscoped.maximum(:id) || 1 sql = "COPY tags (#{columns.map { |c| "\"#{c}\"" }.join(', ')}) FROM STDIN" @raw_connection.copy_data(sql, @encoder) do @@ -366,7 +366,7 @@ class BulkImport::DiscourseMerger < BulkImport::Base puts "merging badges..." columns = Badge.columns.map(&:name) imported_ids = [] - last_id = Badge.unscoped.maximum(:id) + last_id = Badge.unscoped.maximum(:id) || 1 sql = "COPY badges (#{columns.map { |c| "\"#{c}\"" }.join(', ')}) FROM STDIN" @raw_connection.copy_data(sql, @encoder) do From fb9e3e64237e30effb0f845ad6669c2744532936 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Mon, 17 Sep 2018 17:39:46 +0200 Subject: [PATCH 07/56] Update aws-sdk-s3 In preparation for https://github.com/discourse/discourse/pull/6345 --- Gemfile.lock | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 69567ed4a4..b2bf1e9582 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -44,20 +44,20 @@ GEM arel (9.0.0) ast (2.4.0) aws-eventstream (1.0.1) - aws-partitions (1.92.0) - aws-sdk-core (3.21.2) + aws-partitions (1.104.0) + aws-sdk-core (3.27.0) aws-eventstream (~> 1.0) aws-partitions (~> 1.0) aws-sigv4 (~> 1.0) jmespath (~> 1.0) - aws-sdk-kms (1.5.0) - aws-sdk-core (~> 3) + aws-sdk-kms (1.9.0) + aws-sdk-core (~> 3, >= 3.26.0) aws-sigv4 (~> 1.0) - aws-sdk-s3 (1.14.0) - aws-sdk-core (~> 3, >= 3.21.2) + aws-sdk-s3 (1.19.0) + aws-sdk-core (~> 3, >= 3.26.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.0) - aws-sigv4 (1.0.2) + aws-sigv4 (1.0.3) barber (0.12.0) ember-source (>= 1.0, < 3.1) execjs (>= 1.2, < 3) From 6f1b8ad16dab376616a87b1154799af9a9bcdadf Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Mon, 17 Sep 2018 11:40:15 -0400 Subject: [PATCH 08/56] FIX: tag groups page should only be visible to staff No security concern here because nothing private was visible, and no actions could be taken by non-staff users. --- app/controllers/tag_groups_controller.rb | 4 +++- config/routes.rb | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/tag_groups_controller.rb b/app/controllers/tag_groups_controller.rb index 4fd4f0f6e0..d0ee16587c 100644 --- a/app/controllers/tag_groups_controller.rb +++ b/app/controllers/tag_groups_controller.rb @@ -1,5 +1,7 @@ class TagGroupsController < ApplicationController - requires_login except: [:index, :show] + + requires_login + before_action :ensure_staff skip_before_action :check_xhr, only: [:index, :show] before_action :fetch_tag_group, only: [:show, :update, :destroy] diff --git a/config/routes.rb b/config/routes.rb index c661b0a727..0b1e92a36a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -793,7 +793,7 @@ Discourse::Application.routes.draw do end end - resources :tag_groups, except: [:new, :edit] do + resources :tag_groups, constraints: StaffConstraint.new, except: [:new, :edit] do collection do get '/filter/search' => 'tag_groups#search' end From 7f420b61cb8ff01f4ced18acf6d542f177f5ff37 Mon Sep 17 00:00:00 2001 From: Kris Date: Mon, 17 Sep 2018 11:58:16 -0400 Subject: [PATCH 09/56] Removing unneeded theme intro text --- .../javascripts/admin/templates/customize-themes.hbs | 4 +--- app/assets/stylesheets/common/admin/customize.scss | 9 ++------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/admin/templates/customize-themes.hbs b/app/assets/javascripts/admin/templates/customize-themes.hbs index 68f12bc404..6d3e0648cf 100644 --- a/app/assets/javascripts/admin/templates/customize-themes.hbs +++ b/app/assets/javascripts/admin/templates/customize-themes.hbs @@ -1,8 +1,6 @@ {{#unless editingTheme}}
-
-

{{i18n 'admin.customize.theme.long_title'}}

-
+
{{d-button label="admin.customize.new" icon="plus" action="showCreateModal" class="btn-primary"}} diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss index 667393f66c..b03be8f3a3 100644 --- a/app/assets/stylesheets/common/admin/customize.scss +++ b/app/assets/stylesheets/common/admin/customize.scss @@ -64,13 +64,8 @@ padding-bottom: 8px; display: flex; align-items: center; - - .title { - color: $primary-medium; - flex-grow: 1; - h3 { - margin-bottom: 0; - } + .create-actions { + margin-left: auto; } } .admin-container { From 2ff2c4990860a2673adf41c015ceba7a9f625227 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 17 Sep 2018 22:14:41 +0200 Subject: [PATCH 10/56] Revert "FIX: ensures onSelect/onDeselect are called" This reverts commit 52eed7329429abffefb7df7aac469e188a0ae298. --- .../components/single-select.js.es6 | 4 -- .../components/single-select-test.js.es6 | 52 ------------------- 2 files changed, 56 deletions(-) diff --git a/app/assets/javascripts/select-kit/components/single-select.js.es6 b/app/assets/javascripts/select-kit/components/single-select.js.es6 index af3aa3a4f8..7713d51194 100644 --- a/app/assets/javascripts/select-kit/components/single-select.js.es6 +++ b/app/assets/javascripts/select-kit/components/single-select.js.es6 @@ -210,10 +210,6 @@ export default SelectKitComponent.extend({ }, select(computedContentItem) { - if (this.get("hasSelection")) { - this.deselect(this.get("selection.value")); - } - if ( !computedContentItem || computedContentItem.__sk_row_type === "noneRow" diff --git a/test/javascripts/components/single-select-test.js.es6 b/test/javascripts/components/single-select-test.js.es6 index 72366fdf72..439781e202 100644 --- a/test/javascripts/components/single-select-test.js.es6 +++ b/test/javascripts/components/single-select-test.js.es6 @@ -835,55 +835,3 @@ componentTest("without forceEscape", { ); } }); - -componentTest("onSelect", { - template: - "
{{single-select content=content onSelect=(action externalAction)}}", - - beforeEach() { - this.set("externalAction", actual => { - find(".test-external-action").text(actual); - }); - - this.set("content", ["red", "blue"]); - }, - - async test(assert) { - await this.get("subject").expand(); - await this.get("subject").selectRowByValue("red"); - - assert.equal( - find(".test-external-action") - .text() - .trim(), - "red" - ); - } -}); - -componentTest("onDeselect", { - template: - "
{{single-select content=content onDeselect=(action externalAction)}}", - - beforeEach() { - this.set("externalAction", actual => { - find(".test-external-action").text(actual); - }); - - this.set("content", ["red", "blue"]); - }, - - async test(assert) { - await this.get("subject").expand(); - await this.get("subject").selectRowByValue("red"); - await this.get("subject").expand(); - await this.get("subject").selectRowByValue("blue"); - - assert.equal( - find(".test-external-action") - .text() - .trim(), - "red" - ); - } -}); From 0e9841b9951a8bd6f8bdf83b5b13eccdafc675ec Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 18 Sep 2018 08:35:09 +1000 Subject: [PATCH 11/56] SECURITY: remove admin memory diagnostics routes --- .../admin/diagnostics_controller.rb | 42 ----- config/routes.rb | 3 - lib/memory_diagnostics.rb | 169 ------------------ 3 files changed, 214 deletions(-) delete mode 100644 app/controllers/admin/diagnostics_controller.rb delete mode 100644 lib/memory_diagnostics.rb diff --git a/app/controllers/admin/diagnostics_controller.rb b/app/controllers/admin/diagnostics_controller.rb deleted file mode 100644 index 81b076b072..0000000000 --- a/app/controllers/admin/diagnostics_controller.rb +++ /dev/null @@ -1,42 +0,0 @@ -require_dependency 'memory_diagnostics' - -class Admin::DiagnosticsController < Admin::AdminController - layout false - skip_before_action :check_xhr - - def memory_stats - text = nil - - if params.key?(:diff) - if !MemoryDiagnostics.snapshot_exists? - text = "No initial snapshot exists" - else - text = MemoryDiagnostics.compare - end - elsif params.key?(:snapshot) - MemoryDiagnostics.snapshot_current_process - text = "Writing snapshot to: #{MemoryDiagnostics.snapshot_filename}\n\nTo get a diff use ?diff=1" - else - text = MemoryDiagnostics.memory_report(class_report: params.key?(:full)) - end - - render plain: text - end - - def dump_heap - begin - # ruby 2.1 - GC.start(full_mark: true) - require 'objspace' - - io = File.open("discourse-heap-#{SecureRandom.hex(3)}.json", 'w') - ObjectSpace.dump_all(output: io) - io.close - - render plain: "HEAP DUMP:\n#{io.path}" - rescue - render plain: "HEAP DUMP:\nnot supported" - end - end - -end diff --git a/config/routes.rb b/config/routes.rb index 0b1e92a36a..631c0dcaaf 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -290,9 +290,6 @@ Discourse::Application.routes.draw do post "preview" => "badges#preview" end end - - get "memory_stats" => "diagnostics#memory_stats", constraints: AdminConstraint.new - get "dump_heap" => "diagnostics#dump_heap", constraints: AdminConstraint.new end # admin namespace get "email_preferences" => "email#preferences_redirect", :as => "email_preferences_redirect" diff --git a/lib/memory_diagnostics.rb b/lib/memory_diagnostics.rb deleted file mode 100644 index 3a3fcd60f5..0000000000 --- a/lib/memory_diagnostics.rb +++ /dev/null @@ -1,169 +0,0 @@ -module MemoryDiagnostics - - def self.snapshot_exists? - File.exists?(snapshot_filename) - end - - def self.compare(from = nil, to = nil) - - from ||= snapshot_filename - if !to - filename = snapshot_filename + ".new" - snapshot_current_process(filename) - to = filename - end - - from = Marshal::load(IO.binread(from)); - to = Marshal::load(IO.binread(to)); - - diff = from - to - - require 'objspace' - diff = diff.map do |id| - ObjectSpace._id2ref(id) rescue nil - end - diff.compact! - - report = "#{diff.length} objects have leaked\n" - - report << "Summary:\n" - - summary = {} - diff.each do |obj| - begin - summary[obj.class] ||= 0 - summary[obj.class] += 1 - rescue - # don't care - end - end - - report << summary.sort { |a, b| b[1] <=> a[1] }[0..50].map { |k, v| - "#{k}: #{v}" - }.join("\n") - - report << "\n\nSample Items:\n" - - diff[0..5000].each do |v| - report << "#{v.class}: #{String === v ? v[0..300] : (40 + ObjectSpace.memsize_of(v)).to_s + " bytes"}\n" rescue nil - end - - report - end - - def self.snapshot_path - "#{Rails.root}/tmp/mem_snapshots" - end - - def self.snapshot_filename - "#{snapshot_path}/#{Process.pid}.snapshot" - end - - def self.snapshot_current_process(filename = nil) - filename ||= snapshot_filename - pid = fork do - snapshot(filename) - end - - Process.wait(pid) - end - - def self.snapshot(filename) - require 'objspace' - FileUtils.mkdir_p snapshot_path - object_ids = [] - - full_gc - - ObjectSpace.each_object do |o| - begin - object_ids << o.object_id - rescue - # skip - end - end - - IO.binwrite(filename, Marshal::dump(object_ids)) - end - - def self.memory_report(opts = {}) - begin - # ruby 2.1 - GC.start(full_mark: true) - rescue - GC.start - end - - classes = {} - large_objects = [] - - if opts[:class_report] - require 'objspace' - ObjectSpace.each_object do |o| - begin - classes[o.class] ||= 0 - classes[o.class] += 1 - if (size = ObjectSpace.memsize_of(o)) > 200 - large_objects << [size, o] - end - rescue - # all sorts of stuff can happen here BasicObject etc. - classes[:unknown] ||= 0 - classes[:unknown] += 1 - end - end - classes = classes.sort { |a, b| b[1] <=> a[1] }[0..40].map { |klass, count| "#{klass}: #{count}" } - - classes << "\nLarge Objects (#{large_objects.length} larger than 200 bytes total size #{large_objects.map { |x, _| x }.sum}):\n" - - classes += large_objects.sort { |a, b| b[0] <=> a[0] }[0..800].map do |size, object| - rval = "#{object.class}: size #{size}" - rval << " " << object.to_s[0..500].gsub("\n", "") if (String === object) || (Regexp === object) - rval << "\n" - rval - end - end - - stats = GC.stat.map { |k, v| "#{k}: #{v}" } - counts = ObjectSpace.count_objects.sort { |a, b| b[1] <=> a[1] }.map { |k, v| "#{k}: #{v}" } - - < 0 ? classes.join("\n") : "Class report omitted use ?full=1 to include it"} - -TEXT - - end - - def self.full_gc - # gc start may not collect everything - GC.start while new_count = decreased_count(new_count) - end - - def self.decreased_count(old) - count = count_objects - if !old || count < old - count - else - nil - end - end - - def self.count_objects - i = 0 - ObjectSpace.each_object do |obj| - i += 1 - end - end -end From 7d6b348d0b6cd85b474a53299dc378d58c70b865 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 18 Sep 2018 08:54:44 +1000 Subject: [PATCH 12/56] SECURITY: correct XSS on long topic titles --- app/models/topic.rb | 8 ++++---- spec/models/topic_spec.rb | 12 +++++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/app/models/topic.rb b/app/models/topic.rb index c0bbb02163..e2e0fb449c 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -221,8 +221,9 @@ class Topic < ActiveRecord::Base unless skip_callbacks ensure_topic_has_a_category end + if title_changed? - write_attribute :fancy_title, Topic.fancy_title(title) + write_attribute(:fancy_title, Topic.fancy_title(title)) end if category_id_changed? || new_record? @@ -346,10 +347,9 @@ class Topic < ActiveRecord::Base end def self.fancy_title(title) - escaped = ERB::Util.html_escape(title) - return unless escaped + return unless escaped = ERB::Util.html_escape(title) fancy_title = Emoji.unicode_unescape(HtmlPrettify.render(escaped)) - fancy_title.length > Topic.max_fancy_title_length ? title : fancy_title + fancy_title.length > Topic.max_fancy_title_length ? escaped : fancy_title end def fancy_title diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index df1b61225d..5e28d0d618 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -296,10 +296,17 @@ describe Topic do expect(topic_image.fancy_title).to eq("Topic with <img src=‘something’> image in its title") end + it "always escapes title" do + topic_script.title = topic_script.title + "x" * Topic.max_fancy_title_length + expect(topic_script.fancy_title).to eq(ERB::Util.html_escape(topic_script.title)) + # not really needed, but just in case + expect(topic_script.fancy_title).not_to include("