Patching ERB rendering of the data-info JSON (#16067)

Prior to this commit, we were somewhat naively rendering Hash style data
attributes in our ERB templates.  By rendering each hash attribute
separately, we were rendering characters that could break the
javascript (e.g. double hack or backslash `"` or `\`).

By moving to this view_object rendering, we leverage Rails's `to_json`
behavior to ensure properly escaped values.  As part of this exercise, I
generalized the method to allow for other places to benefit from this
behavior.

This generalization also helps ensure that we have a more conformant
rendering (e.g. we should always have an :id, :className, and :name
value in our data-info hash).

_Note: I've updated the user's names for Cypress tests as they are more
likely to catch the particular issue than anything else.  I assume that
I'm going to break some cypress tests and will need some help fixing
them._

Closes #15916, #14704

Supersedes #15983

How to test locally:

Assuming you have seeded database (e.g. `rails db:seed`), checkout the
"main" branch.  Then in `rails console` find a user that's written articles:

```ruby
user = Article.last.user

user.update(name: "\\: #{user.name}")

user.articles.each(&:save)
```

Now, again on the "main" branch, start your application (e.g.,
`bin/startup`).

Then get a logged in and a logged out browser session going.  Open your
web inspector and open console.  Then go to the local instances homepage
(e.g., http://localhost:3000) and look for JS errors.

On the main branch, you should see an exception around
`JSON.parse(button.data.info)` (assuming that the `user`'s article is
rendered on the homepage).

Then go to the user's page (e.g. https://localhost:3000/:user-slug) and
look for JS parse errors.

On this PR's branch (e.g.,
`jeremyf/take-two-at-resolving-gh-15916`)
you shouldn't see those console errors.

More importantly, the Follow buttons should work.

app/assets/javascripts/utilities/buildArticleHTML.js

@@ -32,8 +32,8 @@ function buildArticleHTML(article) {
       currentTag = JSON.parse(container.dataset.params).tag;
     }
     if (article.flare_tag && currentTag !== article.flare_tag.name) {
-      flareTag = `<a href="/t/${article.flare_tag.name}" 
-        class="crayons-tag crayons-tag--filled" 
+      flareTag = `<a href="/t/${article.flare_tag.name}"
+        class="crayons-tag crayons-tag--filled"
         style="--tag-bg: ${article.flare_tag.bg_color_hex}1a; --tag-prefix: ${article.flare_tag.bg_color_hex}; --tag-bg-hover: ${article.flare_tag.bg_color_hex}1a; --tag-prefix-hover: ${article.flare_tag.bg_color_hex};"
       >
         <span class="crayons-tag__prefix">#</span>
@@ -170,6 +170,8 @@ function buildArticleHTML(article) {
     // We only show profile preview cards for Posts
     var isArticle = article.class_name === 'Article';
 
+    // We need to be able to set the data-info hash attribute with escaped characters.
+    var name = article.user.name.replace(/[\\"']/g, '\\$&');
     var previewCardContent = `
       <div id="story-author-preview-content-${article.id}" class="profile-preview-card__content crayons-dropdown p-4" data-repositioning-dropdown="true" style="border-top: var(--su-7) solid var(--card-color);" data-testid="profile-preview-card">
         <div class="gap-4 grid">
@@ -182,7 +184,7 @@ function buildArticleHTML(article) {
             </a>
           </div>
           <div class="print-hidden">
-            <button class="crayons-btn follow-action-button whitespace-nowrap follow-user w-100" data-info='{"id": ${article.user_id}, "className": "User", "style": "full", "name": "${article.user.name}"}'>Follow</button>
+            <button class="crayons-btn follow-action-button whitespace-nowrap follow-user w-100" data-info='{"id": ${article.user_id}, "className": "User", "style": "full", "name": "${name}"}'>Follow</button>
           </div>
           <div class="author-preview-metadata-container" data-author-id="${article.user_id}"></div>
         </div>

app/decorators/article_decorator.rb

@@ -1,6 +1,15 @@
 class ArticleDecorator < ApplicationDecorator
   LONG_MARKDOWN_THRESHOLD = 900
 
+  # @return [String] JSON formatted string.
+  #
+  # @example
+  #   > Article.last.decorate.user_data_info_to_json
+  #   => "{\"user_id\":1,\"className\":\"User\",\"style\":\"full\",\"name\":\"Duane \\\"The Rock\\\" Johnson\"}"
+  def user_data_info_to_json
+    DataInfo.to_json(object: cached_user, class_name: "User", id: user_id, style: "full")
+  end
+
   def current_state_path
     published ? "/#{username}/#{slug}" : "/#{username}/#{slug}?preview=#{password}"
   end

app/decorators/notification_decorator.rb

@@ -3,6 +3,9 @@ class NotificationDecorator < ApplicationDecorator
     def class
       Struct.new(:name).new(name)
     end
+
+    # @see ApplicationRecord#class_name
+    alias_method :class_name, :name
   end.freeze
 
   # returns a stub notifiable object with name and id

app/helpers/application_helper.rb

@@ -112,12 +112,7 @@ module ApplicationHelper
     return if followable == Users::DeletedUser
 
     user_follow = followable.instance_of?(User) ? "follow-user" : ""
-    followable_type = if followable.respond_to?(:decorated?) && followable.decorated?
-                        followable.object.class.name
-                      else
-                        followable.class.name
-                      end
-
+    followable_type = followable.class_name
     followable_name = followable.name
 
     tag.button(
@@ -126,12 +121,7 @@ module ApplicationHelper
       name: :button,
       type: :button,
       data: {
-        info: {
-          id: followable.id,
-          className: followable_type,
-          name: followable_name,
-          style: style
-        }
+        info: DataInfo.to_json(object: followable, className: followable_type, style: style)
       },
       class: "crayons-btn follow-action-button whitespace-nowrap #{classes} #{user_follow}",
       aria: {

app/models/application_record.rb

@@ -35,6 +35,16 @@ class ApplicationRecord < ActiveRecord::Base
     false
   end
 
+  # In our view objects, we often ask "What's this object's class's name?"
+  #
+  # We can either first check "Are you decorated?"  If so, ask for the decorated object's class
+  # name.  Or we can add a helper method for that very thing.
+  #
+  # @return [String]
+  def class_name
+    self.class.name
+  end
+
   # Decorate collection with appropriate decorator
   def self.decorate
     decorator_class.decorate_collection(all)

app/models/tag.rb

@@ -82,6 +82,13 @@ class Tag < ActsAsTaggableOn::Tag
   scope :eager_load_serialized_data, -> {}
   scope :supported, -> { where(supported: true) }
 
+  # @return [String]
+  #
+  # @see ApplicationRecord#class_name
+  def class_name
+    self.class.name
+  end
+
   # possible social previews templates for articles with a particular tag
   def self.social_preview_templates
     Rails.root.join("app/views/social_previews/articles").children.map { |ch| File.basename(ch, ".html.erb") }

app/models/users/deleted_user.rb

@@ -24,6 +24,13 @@ module Users
     def self.path() = nil
     def self.tag_line() = nil
 
+    # @return [String]
+    #
+    # @see ApplicationRecord#class_name
+    def self.class_name
+      User.name
+    end
+
     def self.decorate
       self
     end

app/view_objects/data_info.rb

@@ -0,0 +1,36 @@
+# Throughout Forem, we encode JSON information in our Ruby templates.
+# By convention, we often have a data-info attribute that is a Hash.
+#
+# This module provides some much needed character escaping to ensure
+# that our JS that parses the data-info attribute doesn't choke.
+#
+# @see ./app/javascript/packs/followButtons.js for parsing
+module DataInfo
+  # A convenience method to ensure properly escape JSON data attributes.
+  #
+  # @param object [#class, #id, #name] responds to `#class`, `#id`,
+  #        and `#name`, though you can "cheat" and pass the specific
+  #        values to allow for over-riding (or when you have not quite
+  #        well formed objects; looking at ArticleDecorator's
+  #        cached_user.  It's not quite a valid object.)
+  # @param id [String, Integer]
+  # @param class_name [String]
+  # @param name [String]
+  # @param kwargs [Hash] any additional attributes to include in the
+  #        JSON object.  In some cases we include a "style" attribute.
+  #
+  # @return [String] JSON formatted string.
+  #
+  # @example
+  #   > DataInfo.to_json(object: User.first, name: "Duane \"The Rock\" Johnson", id: 1, style: "full")
+  #   => "{\"user_id\":1,\"className\":\"User\",\"style\":\"full\",\"name\":\"Duane \\\"The Rock\\\" Johnson\"}"
+  def self.to_json(object:, id: object.id, class_name: object.class_name, name: object.name, **kwargs)
+    kwargs.merge(
+      {
+        id: id,
+        className: class_name,
+        name: name
+      },
+    ).to_json
+  end
+end

app/views/articles/_single_story.html.erb

@@ -65,7 +65,7 @@
                   <div class="print-hidden">
                     <button
                       class="crayons-btn follow-action-button whitespace-nowrap follow-user w-100"
-                      data-info='{"id": <%= story.user_id %>, "className": "User", "style": "full", "name": "<%= story.cached_user.name %>"}'>
+                      data-info='<%= story.user_data_info_to_json %>'>
                       <%= t("views.users.follow") %>
                     </button>
                   </div>

app/views/organizations/_header.html.erb

@@ -20,7 +20,7 @@
           <button
             id="user-follow-butt"
             class="crayons-btn whitespace-nowrap follow-action-button"
-            data-info='{"id":<%= @user.id %>,"className":"<%= @user.class.name %>", "name": "<%= @user.name %>"}'>
+            data-info='<%= DataInfo.to_json(object: @user) %>'>
             <%= t("views.organizations.follow") %>
           </button>
         </div>

app/views/podcast_episodes/index.html.erb

@@ -20,7 +20,7 @@
               id="user-follow-butt"
               class="crayons-btn crayons-btn--inverted follow-action-button"
               style="min-width: 100px;"
-              data-info='{"id":<%= @podcast.id %>,"className":"<%= @podcast.class.name %>", "name": "<%= @podcast.name %>"}'>
+              data-info='<%= DataInfo.to_json(object: @podcast) %>'>
                 &nbsp;
             </button>
           </p>

app/views/podcast_episodes/show.html.erb

@@ -38,7 +38,7 @@
         <button
           id="user-follow-butt"
           class="crayons-btn follow-action-button"
-          data-info='{"id":<%= @podcast.id %>,"className":"<%= @podcast.class.name %>", "name": "<%= @podcast.name %>"}'>
+          data-info='<%= DataInfo.to_json(object: @podcast) %>'>
           &nbsp;
         </button>
       </h2>

app/views/stories/tagged_articles/index.html.erb

@@ -24,7 +24,7 @@
               <button
                 id="user-follow-butt"
                 class="crayons-btn follow-action-button"
-                data-info='{"id":<%= @tag.id %>,"className":"Tag", "name": "<%= @tag.pretty_name || @tag.name %>"}'>
+                data-info='<%= DataInfo.to_json(object: @tag, class_name: "Tag", name: @tag.pretty_name || @tag.name) %>'>
                 <%= t("views.stories.follow") %>
               </button>
             <% end %>

app/views/tags/index.html.erb

@@ -45,7 +45,7 @@
           <div class="mt-auto">
             <button
               class="crayons-btn crayons-btn--secondary follow-action-button"
-              data-info='{"id":<%= tag.id %>,"className":"Tag", "followStyle":"secondary", "name": "<%= tag.name %>"}'>
+              data-info='<%= DataInfo.to_json(object: tag, class_name: "Tag", followStyle: "secondary") %>'>
               <%= t("views.tags.follow") %>
             </button>
           </div>

app/views/users/show.html.erb

@@ -32,7 +32,7 @@
           </span>
 
           <div class="profile-header__actions">
-              <button id="user-follow-butt" class="crayons-btn whitespace-nowrap follow-action-button follow-user" data-info='{"id":<%= @user.id %>,"className":"<%= @user.class.name %>", "name": "<%= @user.name %>"}'><%= t("views.users.follow") %></button>
+              <button id="user-follow-butt" class="crayons-btn whitespace-nowrap follow-action-button follow-user" data-info='<%= DataInfo.to_json(object: @user) %>'><%= t("views.users.follow") %></button>
         <div class="profile-dropdown ml-2 s:relative hidden" data-username="<%= @user.username %>">
               <button id="user-profile-dropdown" aria-expanded="false" aria-controls="user-profile-dropdownmenu" aria-haspopup="true" class="crayons-btn crayons-btn--ghost-dimmed crayons-btn--icon">
                 <%= crayons_icon_tag("overflow-horizontal", class: "dropdown-icon", title: t("views.users.dropdown")) %>
@@ -110,7 +110,7 @@
                 <p class="fw-bold mb-2">@<%= @user.username %>'s guidelines:</p>
                 <p class="mb-6"><%= @user.setting.inbox_guidelines %></p>
               <% end %>
-              <form id="new-message-form" class="message-form mb-4" data-info='{"id":<%= @user.id %>,"className":"<%= @user.class.name %>","username":"<%= @user.username %>", "showChat":"<%= @user.setting.inbox_type %>"}'>
+              <form id="new-message-form" class="message-form mb-4" data-info='<%= DataInfo.to_json(object: @user, showChat: @user.setting.inbox_type) %>'>
                 <textarea id="new-message" rows="4" cols="70" placeholder="<%= t("views.users.send_pm.placeholder") %>" class="crayons-textfield"></textarea>
                 <button type="submit" class="submit-message crayons-btn"><%= t("views.users.send_pm.submit") %></button>
               </form>

db/seeds.rb

@@ -59,8 +59,10 @@ users_in_random_order = seeder.create_if_none(User, num_users) do
 
   num_users.times do |i|
     fname = Faker::Name.unique.first_name
-    lname = Faker::Name.unique.last_name
-    name = [fname, lname].join(" ")
+    # Including "\\:/" to help with identifying local issues with
+    # character escaping.
+    lname = Faker::Name.unique.last_name + "\\:/"
+    name = [fname, "\"The #{fname}\"", lname].join(" ")
 
     user = User.create!(
       name: name,
@@ -182,7 +184,7 @@ users_in_random_order = seeder.create_if_none(User, num_users) do
 end
 seeder.create_if_doesnt_exist(User, "email", "admin@forem.local") do
   user = User.create!(
-    name: "Admin McAdmin",
+    name: "Admin \"The \\:/ Administrator\" McAdmin",
     email: "admin@forem.local",
     username: "Admin_McAdmin",
     profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")),

spec/decorators/application_decorator_spec.rb

@@ -8,6 +8,14 @@ RSpec.describe ApplicationDecorator, type: :decorator do
     end
   end
 
+  describe "#class_name" do
+    it "delegates to the underlying object" do
+      obj = User.new
+      decorated = described_class.new(obj)
+      expect(decorated.class_name).to eq(obj.class_name)
+    end
+  end
+
   describe "#decorate" do
     it "returns itself" do
       obj = User.new

spec/decorators/article_decorator_spec.rb

@@ -25,6 +25,15 @@ RSpec.describe ArticleDecorator, type: :decorator do
     end
   end
 
+  describe "#user_data_info_to_json" do
+    it "returns an escaped JSON string" do
+      user = build(:user, name: '\: Hello')
+      allow(article).to receive(:cached_user).and_return(user)
+      decorated = article.decorate
+      expect(JSON.parse(decorated.user_data_info_to_json)).to be_a(Hash)
+    end
+  end
+
   describe "#current_state_path" do
     it "returns the path /:username/:slug when published" do
       article = published_article

spec/decorators/notification_decorator_spec.rb

@@ -49,6 +49,7 @@ RSpec.describe NotificationDecorator, type: :decorator do
 
       expect(result.class).to be_a(Struct)
       expect(result.class.name).to eq("User")
+      expect(result.class_name).to eq("User")
     end
   end
 

spec/factories/users.rb

@@ -7,7 +7,11 @@ FactoryBot.define do
   image_path = Rails.root.join("spec/support/fixtures/images/image1.jpeg")
 
   factory :user do
-    name                         { Faker::Name.name }
+    # Creating a name that has includes double quotes and backslashes.
+    # This way we can see if things are parsing correctly.
+    name do
+      "#{Faker::Name.first_name} \"#{Faker::Name.first_name}\" \\:/ #{Faker::Name.last_name}"
+    end
     email                        { generate :email }
     username                     { generate :username }
     profile_image                { Rack::Test::UploadedFile.new(image_path, "image/jpeg") }

spec/models/application_record_spec.rb

@@ -8,6 +8,13 @@ RSpec.describe ApplicationRecord, type: :model do
     end
   end
 
+  describe "#class_name" do
+    it "is expected to be a string" do
+      user = User.new
+      expect(user.class_name).to eq("User")
+    end
+  end
+
   describe "#decorate" do
     it "decorates an object that has a decorator" do
       sponsorship = build(:sponsorship)

spec/models/tag_spec.rb

@@ -3,6 +3,12 @@ require "rails_helper"
 RSpec.describe Tag, type: :model do
   let(:tag) { build(:tag) }
 
+  describe "#class_name" do
+    subject(:class_name) { tag.class_name }
+
+    it { is_expected.to eq("Tag") }
+  end
+
   describe "validations" do
     describe "builtin validations" do
       subject { tag }

spec/models/users/deleted_user_spec.rb

@@ -3,6 +3,12 @@ require "rails_helper"
 RSpec.describe Users::DeletedUser, type: :model do
   subject(:deleted_user) { described_class }
 
+  describe "#class_name" do
+    subject(:class_name) { described_class.class_name }
+
+    it { is_expected.to eq(User.name) }
+  end
+
   it { is_expected.to respond_to(:id) }
   it { is_expected.to respond_to(:deleted?) }
   it { is_expected.to respond_to(:darker_color) }

spec/support/seeds/seeds_e2e.rb

@@ -65,7 +65,7 @@ admin_user = User.find_by(email: "admin@forem.local")
 
 seeder.create_if_doesnt_exist(User, "email", "trusted-user-1@forem.local") do
   user = User.create!(
-    name: "Trusted User 1",
+    name: "Trusted User 1 \\:/",
     email: "trusted-user-1@forem.local",
     username: "trusted_user_1",
     profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")),
@@ -188,7 +188,7 @@ end
 
 seeder.create_if_doesnt_exist(User, "email", "notifications-user@forem.local") do
   user = User.create!(
-    name: "Notifications User",
+    name: "Notifications User \\:/",
     email: "notifications-user@forem.local",
     username: "notifications_user",
     profile_image: File.open(Rails.root.join("app/assets/images/#{rand(1..40)}.png")),

spec/view_objects/data_info_spec.rb

@@ -0,0 +1,41 @@
+require "rails_helper"
+
+RSpec.describe DataInfo, type: :view_object do
+  describe "#to_json" do
+    subject(:results) { described_class.to_json(**parameters) }
+
+    # Including two characters of special purpose, the double hack
+    # (e.g. '"') and the backslash (e.g. '\').  These were causing
+    # upstream problems.
+    let(:user) { build(:user, id: 123, name: "Duane \"The Rock\" Johnson \\\\.//") }
+
+    context "when given a User" do
+      let(:parameters) { { object: user } }
+
+      it "parses to valid JSON" do
+        expect(JSON.parse(results))
+          .to eq({ "className" => "User", "id" => 123, "name" => "Duane \"The Rock\" Johnson \\\\.//" })
+      end
+    end
+
+    context "when given a User with additional attributes" do
+      let(:parameters) { { object: user, id: 8_675_309, style: "full" } }
+
+      it "parses to valid JSON" do
+        expect(JSON.parse(results))
+          .to eq({ "className" => "User", "id" => 8_675_309, "name" => "Duane \"The Rock\" Johnson \\\\.//",
+                   "style" => "full" })
+      end
+    end
+
+    context "when given a Tag" do
+      let(:tag) { build(:tag, id: 1234) }
+      let(:parameters) { { object: tag } }
+
+      it "parses to valid JSON" do
+        expect(JSON.parse(results))
+          .to eq({ "className" => "Tag", "id" => tag.id, "name" => tag.name })
+      end
+    end
+  end
+end