diff --git a/saml-core/src/main/java/org/keycloak/saml/common/parsers/AbstractParser.java b/saml-core/src/main/java/org/keycloak/saml/common/parsers/AbstractParser.java index 7e7daa5f0b..f2fa8453a6 100755 --- a/saml-core/src/main/java/org/keycloak/saml/common/parsers/AbstractParser.java +++ b/saml-core/src/main/java/org/keycloak/saml/common/parsers/AbstractParser.java @@ -146,7 +146,7 @@ public abstract class AbstractParser implements ParserNamespaceSupport { } private boolean valid(String str) { - return str != null && str.length() > 0; + return str != null && ! str.isEmpty(); } }); diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/util/SAMLParserUtil.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/util/SAMLParserUtil.java index b1ed2782f9..a58cdd3b22 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/util/SAMLParserUtil.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/util/SAMLParserUtil.java @@ -308,6 +308,11 @@ public class SAMLParserUtil { return parseNameIDType(xmlEventReader); } } else if (xmlEvent instanceof EndElement) { + // consume the end element tag + EndElement end = StaxParserUtil.getNextEndElement(xmlEventReader); + String endElementTag = StaxParserUtil.getEndElementName(end); + if (! StaxParserUtil.matches(end, JBossSAMLConstants.ATTRIBUTE_VALUE.get())) + throw logger.parserUnknownEndElement(endElementTag); return ""; } diff --git a/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java b/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java index 713a5bdfe8..2f2157c098 100644 --- a/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java +++ b/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java @@ -200,4 +200,20 @@ public class SAMLParserTest { assertThat(parsedObject, instanceOf(EntityDescriptorType.class)); } } + + @Test + public void testEmptyAttributeValue() throws Exception { + try (InputStream st = SAMLParserTest.class.getResourceAsStream("KEYCLOAK-4790-Empty-attribute-value.xml")) { + Object parsedObject = parser.parse(st); + assertThat(parsedObject, instanceOf(ResponseType.class)); + } + } + + @Test + public void testEmptyAttributeValueLast() throws Exception { + try (InputStream st = SAMLParserTest.class.getResourceAsStream("KEYCLOAK-4790-Empty-attribute-value-last.xml")) { + Object parsedObject = parser.parse(st); + assertThat(parsedObject, instanceOf(ResponseType.class)); + } + } } diff --git a/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-4790-Empty-attribute-value-last.xml b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-4790-Empty-attribute-value-last.xml new file mode 100644 index 0000000000..efd56dfc78 --- /dev/null +++ b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-4790-Empty-attribute-value-last.xml @@ -0,0 +1,35 @@ + + https://x/ + + + + + https://x/ + + C=c,OU=ou + + + + + + + https://x/auth/realms/administration + + + + + aa + + + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:X509 + + + + diff --git a/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-4790-Empty-attribute-value.xml b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-4790-Empty-attribute-value.xml new file mode 100644 index 0000000000..b4b03a1d36 --- /dev/null +++ b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/KEYCLOAK-4790-Empty-attribute-value.xml @@ -0,0 +1,35 @@ + + https://x/ + + + + + https://x/ + + C=c,OU=ou + + + + + + + https://x/auth/realms/administration + + + + + + + + aa + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:X509 + + + +