Skip to content

Commit ea8e552

Browse files
committed
introduce thread safety to saml schema validation:
* per the references below, use of `Dir.chdir` is not thread-safe. this usage was causing exceptions to be raised when running on Puma and in other multi-threaded environments. * this patch also moves the schema read up to a class instance, as this data is static and does not need to be read every time an assertion is validated. this boosts performance, especially in environments with higher throughput. thanks to @dannyb for the assistance! References: * https://www.ruby-forum.com/topic/165079 * https://bugs.ruby-lang.org/issues/9785 * http://www.justskins.com/forums/working-directory-in-thread-42304.html * http://www.ruby-doc.org/core-2.1.5/Dir.html#method-c-chdir
1 parent 06e3312 commit ea8e552

File tree

2 files changed

+27
-21
lines changed

2 files changed

+27
-21
lines changed

lib/onelogin/ruby-saml/response.rb

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -151,19 +151,18 @@ def validate_success_status(soft = true)
151151
end
152152

153153
def validate_structure(soft = true)
154-
Dir.chdir(File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'schemas'))) do
155-
@schema = Nokogiri::XML::Schema(IO.read('saml-schema-protocol-2.0.xsd'))
156-
@xml = Nokogiri::XML(self.document.to_s)
157-
end
158-
if soft
159-
@schema.validate(@xml).map{
160-
@errors << "Schema validation failed";
161-
return false
162-
}
163-
else
164-
@schema.validate(@xml).map{ |error| @errors << "#{error.message}\n\n#{@xml.to_s}";
165-
validation_error("#{error.message}\n\n#{@xml.to_s}")
166-
}
154+
xml = Nokogiri::XML(self.document.to_s)
155+
156+
SamlMessage.schema.validate(xml).map do |error|
157+
if soft
158+
@errors << "Schema validation failed"
159+
break false
160+
else
161+
error_message = [error.message, xml.to_s].join("\n\n")
162+
163+
@errors << error_message
164+
validation_error(error_message)
165+
end
167166
end
168167
end
169168

lib/onelogin/ruby-saml/saml_message.rb

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
require 'cgi'
22
require 'zlib'
33
require 'base64'
4+
require "nokogiri"
45
require "rexml/document"
56
require "rexml/xpath"
7+
require "thread"
68

79
module OneLogin
810
module RubySaml
@@ -12,15 +14,20 @@ class SamlMessage
1214
ASSERTION = "urn:oasis:names:tc:SAML:2.0:assertion"
1315
PROTOCOL = "urn:oasis:names:tc:SAML:2.0:protocol"
1416

15-
def valid_saml?(document, soft = true)
16-
Dir.chdir(File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'schemas'))) do
17-
@schema = Nokogiri::XML::Schema(IO.read('saml-schema-protocol-2.0.xsd'))
18-
@xml = Nokogiri::XML(document.to_s)
17+
def self.schema
18+
@schema ||= Mutex.new.synchronize do
19+
Dir.chdir(File.expand_path("../../../schemas", __FILE__)) do
20+
::Nokogiri::XML::Schema(File.read("saml-schema-protocol-2.0.xsd"))
21+
end
1922
end
20-
if soft
21-
@schema.validate(@xml).map{ return false }
22-
else
23-
@schema.validate(@xml).map{ |error| validation_error("#{error.message}\n\n#{@xml.to_s}") }
23+
end
24+
25+
def valid_saml?(document, soft = true)
26+
xml = Nokogiri::XML(document.to_s)
27+
28+
SamlMessage.schema.validate(xml).map do |error|
29+
break false if soft
30+
validation_error("#{error.message}\n\n#{xml.to_s}")
2431
end
2532
end
2633

0 commit comments

Comments
 (0)