Big pass on escaping and urls

lib/flipper/ui.rb

@@ -20,12 +20,17 @@ def self.root
 
     def self.app(flipper = nil, options = {})
       env_key = options.fetch(:env_key, 'flipper')
-      rack_protection_options = options.fetch(:rack_protection, use: :authenticity_token)
+
+      if options.key?(:rack_protection)
+        warn "[DEPRECATION] `rack_protection` option is deprecated. " +
+          "Flipper::UI now only includes Rack::Protection::AuthenticityToken middleware. " +
+          "If you need additional protection, you can add it yourself."
+      end
 
       app = ->(_) { [200, { Rack::CONTENT_TYPE => 'text/html' }, ['']] }
       builder = Rack::Builder.new
       yield builder if block_given?
-      builder.use Rack::Protection, rack_protection_options
+      builder.use Rack::Protection::AuthenticityToken
       builder.use Rack::MethodOverride
       builder.use Flipper::Middleware::SetupEnv, flipper, env_key: env_key
       builder.use Flipper::UI::Middleware, flipper: flipper, env_key: env_key

lib/flipper/ui/action.rb

@@ -12,7 +12,7 @@ module FeatureNameFromRoute
         def feature_name
           @feature_name ||= begin
             match = request.path_info.match(self.class.route_regex)
-            match ? Rack::Utils.unescape(match[:feature_name]) : nil
+            match ? Flipper::UI::Util.unescape(match[:feature_name]) : nil
           end
         end
         private :feature_name
@@ -164,7 +164,7 @@ def json_response(object)
       # location - The String location to set the Location header to.
       def redirect_to(location)
         status 302
-        header 'location', "#{script_name}#{Rack::Utils.escape_path(location)}"
+        header 'location', "#{script_name}#{location}"
         halt [@code, @headers, ['']]
       end
 

lib/flipper/ui/actions/actors_gate.rb

@@ -25,7 +25,7 @@ def post
 
           if values.empty?
             error = "#{value.inspect} is not a valid actor value."
-            redirect_to("/features/#{feature.key}/actors?error=#{error}")
+            redirect_to("/features/#{Flipper::UI::Util.escape feature.key}/actors?error=#{Flipper::UI::Util.escape error}")
           end
 
           values.each do |value|
@@ -39,7 +39,7 @@ def post
             end
           end
 
-          redirect_to("/features/#{feature.key}")
+          redirect_to("/features/#{Flipper::UI::Util.escape feature.key}")
         end
       end
     end

lib/flipper/ui/actions/boolean_gate.rb

@@ -21,7 +21,7 @@ def post
             feature.disable
           end
 
-          redirect_to "/features/#{@feature.key}"
+          redirect_to "/features/#{Flipper::UI::Util.escape @feature.key}"
         end
       end
     end

lib/flipper/ui/actions/features.rb

@@ -45,13 +45,13 @@ def post
 
           if Util.blank?(value)
             error = "#{value.inspect} is not a valid feature name."
-            redirect_to("/features/new?error=#{error}")
+            redirect_to("/features/new?error=#{Flipper::UI::Util.escape error}")
           end
 
           feature = flipper[value]
           feature.add
 
-          redirect_to "/features/#{value}"
+          redirect_to "/features/#{Flipper::UI::Util.escape value}"
         end
       end
     end

lib/flipper/ui/actions/groups_gate.rb

@@ -30,10 +30,10 @@ def post
               feature.disable_group value
             end
 
-            redirect_to("/features/#{feature.key}")
+            redirect_to("/features/#{Flipper::UI::Util.escape feature.key}")
           else
             error = "The group named #{value.inspect} has not been registered."
-            redirect_to("/features/#{feature.key}/groups?error=#{error}")
+            redirect_to("/features/#{Flipper::UI::Util.escape feature.key}/groups?error=#{Flipper::UI::Util.escape error}")
           end
         end
       end

lib/flipper/ui/actions/percentage_of_actors_gate.rb

@@ -19,10 +19,10 @@ def post
             feature.enable_percentage_of_actors params['value']
           rescue ArgumentError => exception
             error = "Invalid percentage of actors value: #{exception.message}"
-            redirect_to("/features/#{@feature.key}?error=#{error}")
+            redirect_to("/features/#{Flipper::UI::Util.escape @feature.key}?error=#{Flipper::UI::Util.escape error}")
           end
 
-          redirect_to "/features/#{@feature.key}"
+          redirect_to "/features/#{Flipper::UI::Util.escape @feature.key}"
         end
       end
     end

lib/flipper/ui/actions/percentage_of_time_gate.rb

@@ -19,10 +19,10 @@ def post
             feature.enable_percentage_of_time params['value']
           rescue ArgumentError => exception
             error = "Invalid percentage of time value: #{exception.message}"
-            redirect_to("/features/#{@feature.key}?error=#{error}")
+            redirect_to("/features/#{Flipper::UI::Util.escape @feature.key}?error=#{Flipper::UI::Util.escape error}")
           end
 
-          redirect_to "/features/#{@feature.key}"
+          redirect_to "/features/#{Flipper::UI::Util.escape @feature.key}"
         end
       end
     end

lib/flipper/ui/util.rb

@@ -1,9 +1,19 @@
+require "rack/utils"
+
 module Flipper
   module UI
     module Util
       # Private: 0x3000: fullwidth whitespace
       NON_WHITESPACE_REGEXP = /[^\s#{[0x3000].pack("U")}]/
 
+      def self.escape(str)
+        Rack::Utils.escape(str)
+      end
+
+      def self.unescape(str)
+        Rack::Utils.unescape(str)
+      end
+
       def self.blank?(str)
         str.to_s !~ NON_WHITESPACE_REGEXP
       end

lib/flipper/ui/views/add_actor.erb

@@ -6,15 +6,15 @@
 
 <div class="card">
   <h4 class="card-header">
-    <a class="link-dark" href="<%= script_name %>/features/<%= @feature.key %>"><%= @feature.key %></a>
+    <a class="link-dark" href="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>"><%= @feature.key %></a>
     /
     Enable Actor
   </h4>
   <div class="card-body">
     <p>
       Turn on this feature for actors.
     </p>
-    <form action="<%= script_name %>/features/<%= @feature.key %>/actors" method="post" class="row">
+    <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/actors" method="post" class="row">
       <%== csrf_input_tag %>
       <input type="hidden" name="operation" value="enable">
       <div class="col"><input type="text" name="value" placeholder="<%= Flipper::UI.configuration.add_actor_placeholder %>" class="form-control"></div>

lib/flipper/ui/views/add_group.erb

@@ -6,7 +6,7 @@
 
 <div class="card">
   <h4 class="card-header">
-    <a class="link-dark" href="<%= script_name %>/features/<%= @feature.key %>"><%= @feature.key %></a>
+    <a class="link-dark" href="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>"><%= @feature.key %></a>
     /
     Enable Group</h4>
   <div class="card-body">
@@ -16,7 +16,7 @@
       <p>
         Turn on this feature for an entire group of actors.
       </p>
-      <form action="<%= script_name %>/features/<%= @feature.key %>/groups" method="post" class="row">
+      <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/groups" method="post" class="row">
         <%== csrf_input_tag %>
         <input type="hidden" name="operation" value="enable">
         <div class="col">

lib/flipper/ui/views/feature.erb

@@ -50,7 +50,7 @@
                 <button class="btn btn-outline-secondary js-toggle-trigger" data-bs-toggle="collapse" data-bs-target="#add-actor">Add an actor</button>
               </div>
               <div class="col toggle-block-when-on">
-                <form action="<%= script_name %>/features/<%= @feature.key %>/actors" method="post" class="row">
+                <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/actors" method="post" class="row">
                   <%== csrf_input_tag %>
                   <input type="hidden" name="operation" value="enable">
                   <div class="col">
@@ -83,7 +83,7 @@
               </div>
               <div class="col col-auto">
                 <% if write_allowed? %>
-                  <form action="<%= script_name %>/features/<%= @feature.key %>/actors" method="post">
+                  <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/actors" method="post">
                     <%== csrf_input_tag %>
                     <input type="hidden" name="operation" value="disable">
                     <input type="hidden" name="value" value="<%= item %>">
@@ -119,7 +119,7 @@
                 <% if @feature.disabled_groups.empty? %>
                   All groups enabled.
                 <% else %>
-                  <form action="<%= script_name %>/features/<%= @feature.key %>/groups" method="post" class="row">
+                  <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/groups" method="post" class="row">
                     <%== csrf_input_tag %>
                     <input type="hidden" name="operation" value="enable">
                     <div class="col">
@@ -150,7 +150,7 @@
               </div>
               <div class="col col-auto">
                 <% if write_allowed? %>
-                  <form action="<%= script_name %>/features/<%= @feature.key %>/groups" method="post">
+                  <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/groups" method="post">
                     <%== csrf_input_tag %>
                     <input type="hidden" name="operation" value="disable">
                     <input type="hidden" name="value" value="<%= item %>">
@@ -185,15 +185,15 @@
           <% if write_allowed? %>
             <div class="card-body border-bottom toggle-block-when-on bg-lightest">
               <div class="row align-items-center">
-                <form action="<%= script_name %>/features/<%= @feature.key %>/percentage_of_actors" method="post" class="col">
+                <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/percentage_of_actors" method="post" class="col">
                   <%== csrf_input_tag %>
                   <div class="btn-group">
                     <% @percentages.each do |number| %>
                       <input type="submit" name="value" value="<%= number %>%" class="btn btn-light" <% if number == @feature.percentage_of_actors_value %>disabled<% end %>>
                     <% end %>
                   </div>
                 </form>
-                <form action="<%= script_name %>/features/<%= @feature.key %>/percentage_of_actors" method="post" class="col-auto row">
+                <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/percentage_of_actors" method="post" class="col-auto row">
                   <%== csrf_input_tag %>
                   <div class="col-auto">
                     <input style="width:5em;" type="number" max="100" min="0" name="value" <% if @feature.percentage_of_actors_value > 0 %>value="<%= @feature.percentage_of_actors_value %>"<% end %> title="Custom percentage (26, 32, etc.)" placeholder="0" class="form-control">
@@ -226,15 +226,15 @@
           <% if write_allowed? %>
             <div class="card-body border-bottom toggle-block-when-on bg-lightest">
               <div class="row align-items-center">
-                <form action="<%= script_name %>/features/<%= @feature.key %>/percentage_of_time" method="post" class="col">
+                <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/percentage_of_time" method="post" class="col">
                   <%== csrf_input_tag %>
                   <div class="btn-group">
                     <% @percentages.each do |number| %>
                       <input type="submit" name="value" value="<%= number %>%" class="btn btn-light" <% if number == @feature.percentage_of_time_value %>disabled<% end %>>
                     <% end %>
                   </div>
                 </form>
-                <form action="<%= script_name %>/features/<%= @feature.key %>/percentage_of_time" method="post" class="col-auto row">
+                <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/percentage_of_time" method="post" class="col-auto row">
                   <%== csrf_input_tag %>
                   <div class="col-auto">
                     <input style="width:5em;" type="number" max="100" min="0" name="value" <% if @feature.percentage_of_time_value > 0 %>value="<%= @feature.percentage_of_time_value %>"<% end %> title="Custom percentage (26, 32, etc.)" placeholder="0" class="form-control">
@@ -249,7 +249,7 @@
 
       <% if write_allowed? %>
         <div class="card-body">
-          <form action="<%= script_name %>/features/<%= @feature.key %>/boolean" method="post">
+          <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>/boolean" method="post">
             <%== csrf_input_tag %>
 
             <div class="row">
@@ -301,7 +301,7 @@
           <p>
             <%= Flipper::UI.configuration.delete.description %>
           </p>
-          <form action="<%= script_name %>/features/<%= @feature.key %>" method="post">
+          <form action="<%= script_name %>/features/<%= Flipper::UI::Util.escape @feature.key %>" method="post">
             <%== csrf_input_tag %>
             <input type="hidden" id="feature_name" name="_feature" value="<%= feature_name %>">
             <input type="hidden" name="_method" value="DELETE">

lib/flipper/ui/views/features.erb

@@ -36,7 +36,7 @@
       </div>
     </div>
     <% @features.each do |feature| %>
-      <a href="<%= "#{script_name}/features/#{feature.key}" %>" class="list-group-item list-group-item-action">
+      <a href="<%= "#{script_name}/features/#{Flipper::UI::Util.escape(feature.key)}" %>" class="list-group-item list-group-item-action">
         <div class="row align-items-center">
           <div class="col-1 text-center">
             <span class="status <%= feature.color_class %>" data-bs-toggle="tooltip" title=<%= feature.state.to_s.capitalize %>></span>

spec/flipper/ui/actions/actors_gate_spec.rb

@@ -35,7 +35,7 @@
     end
 
     it 'renders add new actor form' do
-      form = '<form action="/features/a/b/actors" method="post" class="row">'
+      form = '<form action="/features/a%2Fb/actors" method="post" class="row">'
       expect(last_response.body).to include(form)
     end
   end
@@ -73,7 +73,7 @@
 
       context "when feature name contains space" do
         before do
-          post 'features/sp%20ace/actors',
+          post 'features/sp+ace/actors',
                { 'value' => value, 'operation' => 'enable', 'authenticity_token' => token },
                'rack.session' => session
         end
@@ -84,7 +84,7 @@
 
         it "redirects back to feature" do
           expect(last_response.status).to be(302)
-          expect(last_response.headers['location']).to eq('/features/sp%20ace')
+          expect(last_response.headers['location']).to eq('/features/sp+ace')
         end
       end
 
@@ -114,7 +114,7 @@
 
           it 'redirects back to feature' do
             expect(last_response.status).to be(302)
-            expect(last_response.headers['location']).to eq('/features/search/actors?error=%22%22%20is%20not%20a%20valid%20actor%20value.')
+            expect(last_response.headers['location']).to eq('/features/search/actors?error=%22%22+is+not+a+valid+actor+value.')
           end
         end
 
@@ -123,7 +123,7 @@
 
           it 'redirects back to feature' do
             expect(last_response.status).to be(302)
-            expect(last_response.headers['location']).to eq('/features/search/actors?error=%22%22%20is%20not%20a%20valid%20actor%20value.')
+            expect(last_response.headers['location']).to eq('/features/search/actors?error=%22%22+is+not+a+valid+actor+value.')
           end
         end
       end

spec/flipper/ui/actions/boolean_gate_spec.rb

@@ -43,7 +43,7 @@
 
       it 'redirects back to feature' do
         expect(last_response.status).to be(302)
-        expect(last_response.headers['location']).to eq('/features/sp%20ace')
+        expect(last_response.headers['location']).to eq('/features/sp+ace')
       end
     end
 

spec/flipper/ui/actions/feature_spec.rb

@@ -197,4 +197,18 @@
       expect(last_response.body).to include('a/b')
     end
   end
+
+  describe 'GET /features/:feature with dot dot slash repeated in feature name' do
+    before do
+      get '/features/..%2F..%2F..%2F..%2Fblah'
+    end
+
+    it 'responds with success' do
+      expect(last_response.status).to be(200)
+    end
+
+    it 'renders template' do
+      expect(last_response.body).to include('../../../../blah')
+    end
+  end
 end