Skip to content

Commit cb6f3ae

Browse files
authored
Merge pull request #84 from miszobi/issue/83-fix-concurrent-validation
fixes #83: don't share a SchemaFactory between instances
2 parents 3fcda2c + 366961d commit cb6f3ae

2 files changed

Lines changed: 62 additions & 22 deletions

File tree

core/src/main/java/com/onelogin/saml2/util/SchemaFactory.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,13 @@
1313
* A class that read SAML schemas that will be used to validate XMLs of the OneLogin's Java Toolkit
1414
*/
1515
public abstract class SchemaFactory {
16-
public static final javax.xml.validation.SchemaFactory schemaFactory = javax.xml.validation.SchemaFactory
17-
.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
18-
1916
public static final URL SAML_SCHEMA_METADATA_2_0 = SchemaFactory.class
2017
.getResource("/schemas/saml-schema-metadata-2.0.xsd");
2118
public static final URL SAML_SCHEMA_PROTOCOL_2_0 = SchemaFactory.class
2219
.getResource("/schemas/saml-schema-protocol-2.0.xsd");
2320

2421
public static Schema loadFromUrl(URL schemaUrl) throws SAXException {
25-
return schemaFactory.newSchema(schemaUrl);
22+
return javax.xml.validation.SchemaFactory
23+
.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI).newSchema(schemaUrl);
2624
}
2725
}

core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,38 @@
11
package com.onelogin.saml2.test.authn;
22

3+
import com.onelogin.saml2.authn.SamlResponse;
4+
import com.onelogin.saml2.http.HttpRequest;
5+
import com.onelogin.saml2.model.SamlResponseStatus;
6+
import com.onelogin.saml2.settings.Saml2Settings;
7+
import com.onelogin.saml2.settings.SettingsBuilder;
8+
import com.onelogin.saml2.util.Constants;
9+
import com.onelogin.saml2.util.Util;
10+
import org.hamcrest.Matchers;
11+
import org.joda.time.Instant;
12+
import org.junit.Test;
13+
import org.w3c.dom.Document;
14+
import org.w3c.dom.Node;
15+
import org.w3c.dom.NodeList;
16+
17+
import java.util.ArrayList;
18+
import java.util.HashMap;
19+
import java.util.List;
20+
import java.util.concurrent.CopyOnWriteArrayList;
21+
import java.util.concurrent.ExecutorService;
22+
import java.util.concurrent.Executors;
23+
import java.util.concurrent.TimeUnit;
24+
import java.util.concurrent.atomic.AtomicInteger;
25+
326
import static org.hamcrest.CoreMatchers.containsString;
427
import static org.hamcrest.CoreMatchers.not;
528
import static org.hamcrest.Matchers.contains;
29+
import static org.hamcrest.Matchers.is;
630
import static org.junit.Assert.assertEquals;
731
import static org.junit.Assert.assertFalse;
832
import static org.junit.Assert.assertNull;
933
import static org.junit.Assert.assertThat;
1034
import static org.junit.Assert.assertTrue;
1135

12-
import java.util.ArrayList;
13-
import java.util.HashMap;
14-
import java.util.List;
15-
16-
import org.joda.time.Instant;
17-
import org.junit.Test;
18-
import org.w3c.dom.Document;
19-
import org.w3c.dom.Node;
20-
import org.w3c.dom.NodeList;
21-
22-
import com.onelogin.saml2.authn.SamlResponse;
23-
import com.onelogin.saml2.http.HttpRequest;
24-
import com.onelogin.saml2.model.SamlResponseStatus;
25-
import com.onelogin.saml2.settings.Saml2Settings;
26-
import com.onelogin.saml2.settings.SettingsBuilder;
27-
import com.onelogin.saml2.util.Constants;
28-
import com.onelogin.saml2.util.Util;
29-
3036
public class AuthnResponseTest {
3137
private static final String ACS_URL = "http://localhost:8080/java-saml-jspsample/acs.jsp";
3238

@@ -1127,6 +1133,42 @@ public void testIsValidSubjectConfirmation_multipleIssues() throws Exception {
11271133
"\n[2] SubjectConfirmationData is no longer valid");
11281134
}
11291135

1136+
@Test
1137+
public void testIsValid_multipleThreads() throws Exception {
1138+
// having
1139+
final int jobCount = 100;
1140+
final int threadCount = 5;
1141+
final ExecutorService executor = Executors.newFixedThreadPool(threadCount);
1142+
final List<Throwable> errors = new CopyOnWriteArrayList<>();
1143+
final AtomicInteger successCount = new AtomicInteger();
1144+
1145+
// when
1146+
for (int i = 0; i < jobCount; i++) {
1147+
executor.submit(new Runnable() {
1148+
@Override
1149+
public void run() {
1150+
try {
1151+
Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build();
1152+
settings.setWantAssertionsSigned(false);
1153+
settings.setWantMessagesSigned(true);
1154+
final String samlResponseEncoded = loadSignMessageAndEncode("data/responses/valid_idp_initiated_response.xml");
1155+
1156+
assertResponseValid(settings, samlResponseEncoded, true, true, null);
1157+
successCount.incrementAndGet();
1158+
} catch (Throwable e) {
1159+
errors.add(e);
1160+
}
1161+
}
1162+
});
1163+
}
1164+
executor.shutdown();
1165+
executor.awaitTermination(30, TimeUnit.SECONDS);
1166+
1167+
// then
1168+
assertThat(errors, Matchers.empty());
1169+
assertThat(successCount.get(), is(jobCount));
1170+
}
1171+
11301172
/**
11311173
* Tests the isValid method of SamlResponse
11321174
* Case: Datetime with Miliseconds

0 commit comments

Comments
 (0)