Self Review

Author: makmic

CHANGELOG.md

@@ -8,7 +8,7 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html
 ### Compatible changes
 * Add compatibility with Rails 7.1
 * Add compatibility with HAML 6
-  * NOTE: Don't use HAML 6.0.0. AngularXSS relies on a patch [introduced in 6.0.1](https://github.com/haml/haml/blob/main/CHANGELOG.md#601). Anything newer should be fine - the gem is tested against 6.3 
+  * NOTE: Don't use HAML 6.0.0. AngularXSS relies on a patch [introduced in 6.0.1](https://github.com/haml/haml/blob/main/CHANGELOG.md#601). Anything newer should be fine - the gem is currently tested against HAML 6.3
 
 ### Breaking changes
 

lib/angular_xss/erb.rb

@@ -1,6 +1,5 @@
 # Use module_eval so we crash when ERB::Util has not yet been loaded.
 if defined?(ActiveSupport::CoreExt::ERBUtil) && ERB::Util.is_a?(ActiveSupport::CoreExt::ERBUtil)
-
   # Rails 7.1+
   # https://github.com/rails/rails/blob/main/activesupport/lib/active_support/core_ext/erb/util.rb
   module ERBUtilExt

lib/angular_xss/output_buffer.rb

@@ -7,14 +7,19 @@
 
 if defined?(ActionView::VERSION) && Gem::Version.new(ActionView::VERSION::STRING) >= Gem::Version.new('7.1')
   # ActionView < 7.1 used our patched ERB::Util.h to escape, 7.1 switched to CGI.escapeHTML
-  module OutputBufferExt
+  module OutputBufferWithEscapedAngularXSS
     def <<(value)
       super(AngularXss::Escaper.escape_if_unsafe(value))
     end
 
-    alias :concat :<<
-    alias :append= :<<
+    def concat(value)
+      super(AngularXss::Escaper.escape_if_unsafe(value))
+    end
+
+    def append=(value)
+      super(AngularXss::Escaper.escape_if_unsafe(value))
+    end
   end
 
-  ActionView::OutputBuffer.prepend OutputBufferExt
+  ActionView::OutputBuffer.prepend OutputBufferWithEscapedAngularXSS
 end

lib/angular_xss/version.rb

@@ -1,3 +1,3 @@
 module AngularXss
-  VERSION = '0.4.1' # todo
+  VERSION = '0.4.1'
 end

spec/angular_xss/erb_spec.rb

@@ -1,9 +1,5 @@
-require 'spec_helper'
-
 describe 'Angular XSS prevention in ERB', :type => :view do
-
   it_should_behave_like 'engine preventing Angular XSS', :partial => 'test_erb'
-
 end
 
 describe ERB::Util do

spec/angular_xss/escaper_spec.rb

@@ -0,0 +1,21 @@
+describe AngularXss::Escaper do
+  describe '.escape' do
+    it 'replaces double brackets with a closed variant' do
+      expect(described_class.escape('{{')).to eq('{{ $root.DOUBLE_LEFT_CURLY_BRACE }}')
+    end
+
+    it 'does not handle HTML safe strings differently' do
+      expect(described_class.escape('{{'.html_safe)).to eq('{{ $root.DOUBLE_LEFT_CURLY_BRACE }}')
+    end
+  end
+
+  describe '.escape_if_unsafe' do
+    it 'replaces double brackets with a closed variant' do
+      expect(described_class.escape_if_unsafe('{{')).to eq('{{ $root.DOUBLE_LEFT_CURLY_BRACE }}')
+    end
+
+    it 'does not modify HTML safe strings' do
+      expect(described_class.escape_if_unsafe('{{'.html_safe)).to eq('{{')
+    end
+  end
+end

spec/angular_xss/haml_spec.rb

@@ -1,5 +1,3 @@
-require 'spec_helper'
-
 describe 'Angular XSS prevention in Haml', :type => :view do
 
   it_should_behave_like 'engine preventing Angular XSS', :partial => 'test_haml'

spec/angular_xss/output_buffer_spec.rb

@@ -1,19 +1,45 @@
-require 'spec_helper'
-
 describe ActionView::OutputBuffer do
   describe '#<<' do
     it 'escapes angular brackets' do
       expect((subject << "{{unsafe}}").to_s).to eq("{{ $root.DOUBLE_LEFT_CURLY_BRACE }}unsafe}}")
     end
 
-    it 'still allows concatting nil' do
+    it 'does not change behavior for already HTML safe strings' do
+      expect((subject << "{{safe}}".html_safe).to_s).to eq("{{safe}}")
+    end
+
+    it 'allows concatting nil' do
       expect { subject << nil }.to_not raise_error
     end
+  end
+
+  describe '#concat' do
+    it 'escapes angular brackets' do
+      expect((subject.concat "{{unsafe}}").to_s).to eq("{{ $root.DOUBLE_LEFT_CURLY_BRACE }}unsafe}}")
+    end
 
     it 'does not change behavior for already HTML safe strings' do
-      expect { subject << nil }.to_not raise_error
-      expect((subject << "{{safe}}".html_safe).to_s).to eq("{{safe}}")
+      expect((subject.concat "{{safe}}".html_safe).to_s).to eq("{{safe}}")
     end
 
+    it 'allows concatting nil' do
+      expect { subject.concat nil }.to_not raise_error
+    end
+  end
+
+  describe '#append=' do
+    it 'escapes angular brackets' do
+      subject.append = "{{unsafe}}"
+      expect(subject.to_s).to eq("{{ $root.DOUBLE_LEFT_CURLY_BRACE }}unsafe}}")
+    end
+
+    it 'does not change behavior for already HTML safe strings' do
+      subject.append = "{{safe}}".html_safe
+      expect(subject.to_s).to eq("{{safe}}")
+    end
+
+    it 'allows concatting nil' do
+      expect { subject.append = nil }.to_not raise_error
+    end
   end
 end

spec/angular_xss/safe_buffer_spec.rb

@@ -1,9 +1,21 @@
-require 'spec_helper'
-
 describe ActiveSupport::SafeBuffer do
 
-  it 'still allows concatting nil' do
-    expect { subject << nil }.to_not raise_error
+  describe '#<<' do
+    it 'escapes angular brackets' do
+      subject << "{{unsafe}}"
+      expect(subject.to_s).to eq("{{ $root.DOUBLE_LEFT_CURLY_BRACE }}unsafe}}")
+    end
+
+    it 'allows concatting nil' do
+      expect { subject << nil }.to_not raise_error
+    end
+  end
+
+  describe '#+' do
+    it 'escapes angular brackets' do
+      combined_string = subject + "{{unsafe}}"
+      expect(combined_string.to_s).to eq("{{ $root.DOUBLE_LEFT_CURLY_BRACE }}unsafe}}")
+    end
   end
 
 end

spec/spec_helper.rb

@@ -29,13 +29,3 @@ def self.env
 Dir["#{File.dirname(__FILE__)}/support/**/*.rb"].each {|f| require f}
 
 TEMPLATE_ROOT = Pathname.new(__dir__).join('templates')
-
-
-RSpec.configure do |config|
-  config.mock_with :rspec do |c|
-    c.syntax = [:should, :expect]
-  end
-  config.expect_with :rspec do |c|
-    c.syntax = [:should, :expect]
-  end
-end