Use a configuration object instead of a bare hash

sparklemotion · Aug 21, 2018 · d547d5c · d547d5c
1 parent c9e6f8f
commit d547d5c
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 22 deletions.
diff --git a/ext/nokogiri/xslt_stylesheet.c b/ext/nokogiri/xslt_stylesheet.c
@@ -8,11 +8,6 @@
 
 VALUE xslt;
 
-enum xsltSecurityAction {
-  XSLT_SEC_FORBID = 1,
-  XSLT_SEC_ALLOW = 2
-};
-
 int vasprintf (char **strp, const char *fmt, va_list ap);
 void vasprintf_free (void *p);
 
@@ -257,13 +252,11 @@ static VALUE registr(VALUE self, VALUE uri, VALUE obj)
 
 int add_sec_pref(VALUE key, VALUE val, VALUE in)
 {
-  Check_Type(key, T_FIXNUM);
-  Check_Type(val, T_FIXNUM);
   xsltSecurityPrefsPtr xsltPrefs = (xsltSecurityPrefsPtr) in;
-  if(NUM2INT(val) == XSLT_SEC_FORBID) {
-    xsltSetSecurityPrefs(xsltPrefs, NUM2INT(key), xsltSecurityForbid);
-  } else if(NUM2INT(val) == XSLT_SEC_ALLOW) {
+  if(val == Qtrue) {
     xsltSetSecurityPrefs(xsltPrefs, NUM2INT(key), xsltSecurityAllow);
+  } else if(val == Qfalse) {
+    xsltSetSecurityPrefs(xsltPrefs, NUM2INT(key), xsltSecurityForbid);
   }
 
   return ST_CONTINUE;

diff --git a/lib/nokogiri/xslt.rb b/lib/nokogiri/xslt.rb
@@ -36,8 +36,11 @@ def parse string, modules = {}
         end
       end
 
+      ###
+      # Set the default security options used by libxslt
+      # +prefs+ should be an object of type Nokogiri::XSLT::Security::Config
       def set_default_security_prefs prefs
-        Stylesheet.set_default_security_prefs(prefs.map{|k,v| { Security.keys[k] => v}}.reduce(:merge))
+        Stylesheet.set_default_security_prefs(Security.keys.map{|k,v| { v => prefs.send(k)  }}.reduce(:merge))
       end
 
       ###

diff --git a/lib/nokogiri/xslt/security.rb b/lib/nokogiri/xslt/security.rb
@@ -1,18 +1,31 @@
 module Nokogiri
   module XSLT
     module Security
+      class Config
+        attr_accessor :allow_read_file
+        attr_accessor :allow_write_file
+        attr_accessor :allow_create_directory
+        attr_accessor :allow_read_network
+        attr_accessor :allow_write_network
+
+        def initialize
+          @allow_read_file = false
+          @allow_write_file = false
+          @allow_create_directory = false
+          @allow_read_network = false
+          @allow_write_network = false
+        end
+      end
+
       def self.keys
         {
-          READ_FILE: 1,
-          WRITE_FILE: 2,
-          CREATE_DIRECTORY: 3,
-          READ_NETWORK: 4,
-          WRITE_NETWORK: 5
+          allow_read_file: 1,
+          allow_write_file: 2,
+          allow_create_directory: 3,
+          allow_read_network: 4,
+          allow_write_network: 5
         }
       end
-
-      FORBID = 1
-      ALLOW = 2
     end
   end
 end
diff --git a/test/test_xslt_transforms.rb b/test/test_xslt_transforms.rb
@@ -1,7 +1,6 @@
 require "helper"
 
 class TestXsltTransforms < Nokogiri::TestCase
-  include Nokogiri::XSLT::Security
 
   def setup
     @doc = Nokogiri::XML(File.open(XML_FILE))
@@ -197,11 +196,18 @@ def test_quote_params
 
   if Nokogiri.uses_libxml?
     def test_set_default_security_prefs
-      Nokogiri::XSLT.set_default_security_prefs({ READ_FILE: FORBID})
+      # Default should be secure
+      sec_prefs = Nokogiri::XSLT::Security::Config.new
+      Nokogiri::XSLT.set_default_security_prefs(sec_prefs)
       assert_raises(RuntimeError) { Nokogiri::XSLT(File.open(XSLT_INCLUDING_FILE)) }
 
-      Nokogiri::XSLT.set_default_security_prefs({ READ_FILE: ALLOW})
+      sec_prefs.allow_read_file = true
+      Nokogiri::XSLT.set_default_security_prefs(sec_prefs)
       assert doc = Nokogiri::XSLT(File.open(XSLT_INCLUDING_FILE))
+
+      sec_prefs.allow_read_file = false
+      Nokogiri::XSLT.set_default_security_prefs(sec_prefs)
+      assert_raises(RuntimeError) { Nokogiri::XSLT(File.open(XSLT_INCLUDING_FILE)) }
     end
   end