Skip to content

Commit

Permalink
Reverting modifications in the FilterParser which incorrectly convert…
Browse files Browse the repository at this point in the history 
…ed the return value of readValue to a String. The original method signature returning Object was restored so the evaluation of isQuoteFilterValue() is accurate.

===BEGIN-RELEASE-NOTE===
Addressed an issue in SCIM filter processing where the parser may evaluate compound statements as a quoted value.
===END-RELEASE-NOTE===

Reviewer: Andy Coulbeck
Reviewer: Dan Vernon
Reviewer: Doug Bulkley

JiraIssue:DS-17454

Product:ds
Product:proxy
Product:sync
  • Loading branch information
@richardcardona
richardcardona committed Mar 10, 2017
1 parent 64b1dad commit c66701f
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 46 deletions.
11 changes: 11 additions & 0 deletions resource/Release-Notes.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@ https://www.unboundid.com/resources/scim

* Resolved Issues

---------- SCIM 1.8.17-SNAPSHOT ----------

* Release Date: TBD

* New Features

* Resolved Issues

- Addressed an issue in SCIM filter processing where the parser may evaluate
compound statements as a quoted value.

---------- SCIM 1.8.16 ----------

* Release Date: February 13, 2017
Expand Down
97 changes: 61 additions & 36 deletions scim-ldap/src/test/java/com/unboundid/scim/ldap/UserResourceMapperTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.fail;

import java.io.ByteArrayOutputStream;
Expand Down Expand Up @@ -306,16 +307,18 @@ public void testFilterMappings()
{
final ResourceMapper mapper = getUserResourceMapper();

Filter filter = mapper.toLDAPFilter(
SCIMFilter.parse("userName eq \"test\""), null);
SCIMFilter scimFilter = SCIMFilter.parse("userName eq \"test\"");
assertTrue(scimFilter.isQuoteFilterValue());
Filter filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_EQUALITY);
assertEquals(filter.getAttributeName(), "uid");
assertEquals(filter.getAssertionValue(), "test");

filter = mapper.toLDAPFilter(
SCIMFilter.parse("name.formatted lt \"test\""), null);
scimFilter = SCIMFilter.parse("name.formatted lt \"test\"");
assertTrue(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
Filter andFilter = filter.getComponents()[0];
assertEquals(andFilter.getFilterType(), Filter.FILTER_TYPE_AND);
Expand All @@ -330,26 +333,31 @@ public void testFilterMappings()
assertEquals(filter.getAttributeName(), "cn");
assertEquals(filter.getAssertionValue(), "test");

filter = mapper.toLDAPFilter(
SCIMFilter.parse("name.familyName ge " +
"\"2015-02-06T09:37:57.969-07:00\""), null);

scimFilter =
SCIMFilter.parse("name.familyName ge \"2015-02-06T09:37:57.969-07:00\"");
assertTrue(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_GREATER_OR_EQUAL);
assertEquals(filter.getAttributeName(), "sn");
assertEquals(filter.getAssertionValue(), "2015-02-06T09:37:57.969-07:00");

filter = mapper.toLDAPFilter(
SCIMFilter.parse("name.familyName ge \"2015-02-06\""), null);
scimFilter =
SCIMFilter.parse("name.familyName ge \"2015-02-06\"");
assertTrue(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_GREATER_OR_EQUAL);
assertEquals(filter.getAttributeName(), "sn");
assertEquals(filter.getAssertionValue(), "2015-02-06");

filter = mapper.toLDAPFilter(
SCIMFilter.parse("meta.lastModified lt " +
"\"2015-02-06T09:37:57.969-07:00\""), null);
scimFilter = SCIMFilter.parse("meta.lastModified lt " +
"\"2015-02-06T09:37:57.969-07:00\"");
assertTrue(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
andFilter = filter.getComponents()[0];
assertEquals(andFilter.getFilterType(), Filter.FILTER_TYPE_AND);
Expand All @@ -364,36 +372,44 @@ public void testFilterMappings()
assertEquals(filter.getAttributeName(), "modifyTimestamp");
assertEquals(filter.getAssertionValue(), "20150206163757.969Z");

filter = mapper.toLDAPFilter(
SCIMFilter.parse("name.familyName ge \"test\""), null);
scimFilter =
SCIMFilter.parse("name.familyName ge \"test\"");
assertTrue(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_GREATER_OR_EQUAL);
assertEquals(filter.getAttributeName(), "sn");
assertEquals(filter.getAssertionValue(), "test");

// No mapping for name.middleName so we should get match nothing
filter = mapper.toLDAPFilter(
SCIMFilter.parse("name.middleName eq \"test\""), null);
scimFilter =
SCIMFilter.parse("name.middleName eq \"test\"");
assertTrue(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertNull(filter);

filter = mapper.toLDAPFilter(SCIMFilter.parse("name.givenName pr"), null);
scimFilter = SCIMFilter.parse("name.givenName pr");
assertFalse(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_PRESENCE);
assertEquals(filter.getAttributeName(), "givenName");
assertNull(filter.getAssertionValue());

filter = mapper.toLDAPFilter(SCIMFilter.parse("emails eq \"test\""), null);
scimFilter = SCIMFilter.parse("emails eq \"test\"");
assertTrue(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_EQUALITY);
assertEquals(filter.getAttributeName(), "mail");
assertEquals(filter.getAssertionValue(), "test");

filter = mapper.toLDAPFilter(
SCIMFilter.parse("phoneNumbers eq \"tel:+1-512-456-7890;ext=123\""),
null);
scimFilter =
SCIMFilter.parse("phoneNumbers eq \"tel:+1-512-456-7890;ext=123\"");
assertTrue(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_OR);
Expand All @@ -412,7 +428,9 @@ public void testFilterMappings()

try
{
mapper.toLDAPFilter(SCIMFilter.parse("addresses eq \"test\""), null);
scimFilter = SCIMFilter.parse("addresses eq \"test\"");
assertTrue(scimFilter.isQuoteFilterValue());
mapper.toLDAPFilter(scimFilter, null);
fail("The complex filter attribute 'addresses' without a sub-attribute " +
"should cause an exception to be thrown");
}
Expand All @@ -421,8 +439,10 @@ public void testFilterMappings()
// Expected.
}

filter = mapper.toLDAPFilter(
SCIMFilter.parse("addresses.formatted eq \"test\""), null);
scimFilter =
SCIMFilter.parse("addresses.formatted eq \"test\"");
assertTrue(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_OR);
Expand All @@ -433,31 +453,36 @@ public void testFilterMappings()
assertFilterComponentAttributeNameAndAssertionValue(components,
"homePostalAddress", "test");

filter = mapper.toLDAPFilter(SCIMFilter.parse(
"name.formatted eq \"test\" and userName eq \"test\""), null);
scimFilter = SCIMFilter.parse(
"name.formatted eq \"test\" and userName eq \"test\"");
assertFalse(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
assertEquals(filter.getComponents().length, 2);

filter = mapper.toLDAPFilter(
SCIMFilter.parse("name.formatted eq \"test\" or userName eq \"test\""),
null);
scimFilter =
SCIMFilter.parse("name.formatted eq \"test\" or userName eq \"test\"");
assertFalse(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_OR);
assertEquals(filter.getComponents().length, 2);

// Attributes in the filter w/o mapping and results in match nothing should
// be correctly short-circuited when there is an AND
filter = mapper.toLDAPFilter(
SCIMFilter.parse(
"name.middleName eq \"test\" and userName eq \"test\""), null);
scimFilter = SCIMFilter.parse(
"name.middleName eq \"test\" and userName eq \"test\"");
assertFalse(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertNull(filter);

filter = mapper.toLDAPFilter(
SCIMFilter.parse(
"name.middleName eq \"test\" or userName eq \"test\""), null);
scimFilter = SCIMFilter.parse(
"name.middleName eq \"test\" or userName eq \"test\"");
assertFalse(scimFilter.isQuoteFilterValue());
filter = mapper.toLDAPFilter(scimFilter, null);
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_AND);
filter = filter.getComponents()[0];
assertEquals(filter.getFilterType(), Filter.FILTER_TYPE_OR);
Expand Down
23 changes: 13 additions & 10 deletions scim-sdk/src/main/java/com/unboundid/scim/sdk/FilterParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,11 +355,11 @@ else if (operator.equalsIgnoreCase("le"))
throw new IllegalArgumentException(msg);
}

final String filterValueString;
final Object filterValue;
if (!filterType.equals(SCIMFilterType.PRESENCE))
{
filterValueString = readValue();
if (filterValueString == null)
filterValue = readValue();
if (filterValue == null)
{
final String msg = String.format(
"End of input at position %d while expecting a value for " +
Expand All @@ -369,12 +369,15 @@ else if (operator.equalsIgnoreCase("le"))
}
else
{
filterValueString = null;
filterValue = null;
}

final String filterValueString = (filterValue != null) ?
filterValue.toString() : null;
return new SCIMFilter(
filterType, filterAttribute, filterValueString,
(filterValueString != null), null);
filterType, filterAttribute,
filterValueString,
(filterValue != null) && (filterValue instanceof String),
null);
}


Expand Down Expand Up @@ -577,10 +580,10 @@ private void rewind()
* after the value is consumed. The start of the value is saved in
* {@code markPos}.
*
* @return A String representing the value at the current position, or
* @return An Object representing the value at the current position, or
* {@code null} if the end of the input has already been reached.
*/
public String readValue()
public Object readValue()
{
skipWhitespace();
markPos = currentPos;
Expand Down Expand Up @@ -792,7 +795,7 @@ public String readValue()
throw new IllegalArgumentException(msg);
}

return s;
return value;
}
}

Expand Down

0 comments on commit c66701f

Please sign in to comment.