From 8a5b1bfefce83141ca4887466f02247d9e857164 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 13 Feb 2018 20:18:49 +0100 Subject: [PATCH] SONAR-10356 ComponentDao calls fail if too many conditions --- .../main/java/org/sonar/db/DatabaseUtils.java | 11 +++ .../java/org/sonar/db/DatabaseUtilsTest.java | 16 ++++ .../org/sonar/db/component/ComponentDao.java | 29 ++++++- .../sonar/db/component/ComponentDaoTest.java | 86 +++++++++++++++++-- 4 files changed, 136 insertions(+), 6 deletions(-) diff --git a/server/sonar-db-core/src/main/java/org/sonar/db/DatabaseUtils.java b/server/sonar-db-core/src/main/java/org/sonar/db/DatabaseUtils.java index 975563da4ec..a53a0b317c5 100644 --- a/server/sonar-db-core/src/main/java/org/sonar/db/DatabaseUtils.java +++ b/server/sonar-db-core/src/main/java/org/sonar/db/DatabaseUtils.java @@ -47,6 +47,7 @@ import javax.annotation.Nullable; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.Lists.newArrayList; import static java.lang.String.format; @@ -338,4 +339,14 @@ public class DatabaseUtils { } }; } + + /** + * @throws IllegalArgumentException if the collection is not null and has strictly more + * than {@link #PARTITION_SIZE_FOR_ORACLE} values. + */ + public static void checkThatNotTooManyConditions(@Nullable Collection values, String message) { + if (values != null) { + checkArgument(values.size() <= PARTITION_SIZE_FOR_ORACLE, message); + } + } } diff --git a/server/sonar-db-core/src/test/java/org/sonar/db/DatabaseUtilsTest.java b/server/sonar-db-core/src/test/java/org/sonar/db/DatabaseUtilsTest.java index 49060d8bf60..3bc10936e61 100644 --- a/server/sonar-db-core/src/test/java/org/sonar/db/DatabaseUtilsTest.java +++ b/server/sonar-db-core/src/test/java/org/sonar/db/DatabaseUtilsTest.java @@ -332,4 +332,20 @@ public class DatabaseUtilsTest { assertThat(DatabaseUtils.tableExists("foo", connection)).isFalse(); } } + + @Test + public void checkThatNotTooManyConditions_does_not_fail_if_less_than_1000_conditions() { + DatabaseUtils.checkThatNotTooManyConditions(null, "unused"); + DatabaseUtils.checkThatNotTooManyConditions(Collections.emptySet(), "unused"); + DatabaseUtils.checkThatNotTooManyConditions(Collections.nCopies(10, "foo"), "unused"); + DatabaseUtils.checkThatNotTooManyConditions(Collections.nCopies(1_000, "foo"), "unused"); + } + + @Test + public void checkThatNotTooManyConditions_throws_IAE_if_strictly_more_than_1000_conditions() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("the message"); + + DatabaseUtils.checkThatNotTooManyConditions(Collections.nCopies(1_001, "foo"), "the message"); + } } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentDao.java index b52280819cf..1698bb36951 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/component/ComponentDao.java @@ -46,6 +46,7 @@ import static java.util.Objects.requireNonNull; import static org.apache.commons.lang.StringUtils.isBlank; import static org.sonar.core.util.stream.MoreCollectors.toList; import static org.sonar.db.DaoDatabaseUtils.buildLikeValue; +import static org.sonar.db.DatabaseUtils.checkThatNotTooManyConditions; import static org.sonar.db.DatabaseUtils.executeLargeInputs; import static org.sonar.db.DatabaseUtils.executeLargeUpdates; import static org.sonar.db.WildcardPosition.BEFORE_AND_AFTER; @@ -57,6 +58,7 @@ public class ComponentDao implements Dao { if (query.hasEmptySetOfComponents()) { return emptyList(); } + checkThatNotTooManyComponents(query); return mapper(session).selectByQuery(organizationUuid, query, new RowBounds(offset, limit)); } @@ -64,7 +66,7 @@ public class ComponentDao implements Dao { if (query.hasEmptySetOfComponents()) { return 0; } - + checkThatNotTooManyComponents(query); return mapper(session).countByQuery(organizationUuid, query); } @@ -104,19 +106,37 @@ public class ComponentDao implements Dao { return componentDto.get(); } + /** + * Same as {@link #selectByQuery(DbSession, String, ComponentQuery, int, int)} except + * that the filter on organization is disabled. + */ public List selectByQuery(DbSession session, ComponentQuery query, int offset, int limit) { return selectByQueryImpl(session, null, query, offset, limit); } + /** + * @throws IllegalArgumentException if parameter query#getComponentIds() has more than {@link org.sonar.db.DatabaseUtils#PARTITION_SIZE_FOR_ORACLE} values + * @throws IllegalArgumentException if parameter query#getComponentKeys() has more than {@link org.sonar.db.DatabaseUtils#PARTITION_SIZE_FOR_ORACLE} values + * @throws IllegalArgumentException if parameter query#getComponentUuids() has more than {@link org.sonar.db.DatabaseUtils#PARTITION_SIZE_FOR_ORACLE} values + */ public List selectByQuery(DbSession dbSession, String organizationUuid, ComponentQuery query, int offset, int limit) { requireNonNull(organizationUuid, "organizationUuid can't be null"); return selectByQueryImpl(dbSession, organizationUuid, query, offset, limit); } + /** + * Same as {@link #countByQuery(DbSession, String, ComponentQuery)} except + * that the filter on organization is disabled. + */ public int countByQuery(DbSession session, ComponentQuery query) { return countByQueryImpl(session, null, query); } + /** + * @throws IllegalArgumentException if parameter query#getComponentIds() has more than {@link org.sonar.db.DatabaseUtils#PARTITION_SIZE_FOR_ORACLE} values + * @throws IllegalArgumentException if parameter query#getComponentKeys() has more than {@link org.sonar.db.DatabaseUtils#PARTITION_SIZE_FOR_ORACLE} values + * @throws IllegalArgumentException if parameter query#getComponentUuids() has more than {@link org.sonar.db.DatabaseUtils#PARTITION_SIZE_FOR_ORACLE} values + */ public int countByQuery(DbSession session, String organizationUuid, ComponentQuery query) { requireNonNull(organizationUuid, "organizationUuid can't be null"); return countByQueryImpl(session, organizationUuid, query); @@ -347,4 +367,11 @@ public class ComponentDao implements Dao { public List selectComponentKeysHavingIssuesToMerge(DbSession dbSession, String mergeBranchUuid) { return mapper(dbSession).selectComponentKeysHavingIssuesToMerge(mergeBranchUuid); } + + private static void checkThatNotTooManyComponents(ComponentQuery query) { + checkThatNotTooManyConditions(query.getComponentIds(), "Too many component ids in query"); + checkThatNotTooManyConditions(query.getComponentKeys(), "Too many component keys in query"); + checkThatNotTooManyConditions(query.getComponentUuids(), "Too many component UUIDs in query"); + } + } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/component/ComponentDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/component/ComponentDaoTest.java index dc110414f9f..d331ad0fb06 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/component/ComponentDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/component/ComponentDaoTest.java @@ -28,6 +28,8 @@ import java.util.Map; import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.LongStream; import javax.annotation.Nullable; import org.assertj.core.api.ListAssert; import org.junit.Rule; @@ -722,7 +724,7 @@ public class ComponentDaoTest { } @Test - public void select_provisioned() { + public void selectByQuery_provisioned() { OrganizationDto organization = db.organizations().insert(); ComponentDto provisionedProject = db.components() .insertComponent(newPrivateProjectDto(organization).setDbKey("provisioned.project").setName("Provisioned Project")); @@ -779,6 +781,51 @@ public class ComponentDaoTest { assertThat(underTest.countByQuery(dbSession, organization.getUuid(), query.get().setQualifiers(Qualifiers.PROJECT, Qualifiers.VIEW).build())).isEqualTo(1); } + @Test + public void countByQuery_with_organization_throws_NPE_of_organizationUuid_is_null() { + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("organizationUuid can't be null"); + + underTest.countByQuery(dbSession, null, ALL_PROJECTS_COMPONENT_QUERY); + } + + @Test + public void countByQuery_throws_IAE_if_too_many_component_ids() { + Set ids = LongStream.range(0L, 1_010L).boxed().collect(Collectors.toSet()); + ComponentQuery.Builder query = ComponentQuery.builder() + .setQualifiers(Qualifiers.PROJECT) + .setComponentIds(ids); + + assertThatCountByQueryThrowsIAE(query, "Too many component ids in query"); + } + + @Test + public void countByQuery_throws_IAE_if_too_many_component_keys() { + Set keys = IntStream.range(0, 1_010).mapToObj(String::valueOf).collect(Collectors.toSet()); + ComponentQuery.Builder query = ComponentQuery.builder() + .setQualifiers(Qualifiers.PROJECT) + .setComponentKeys(keys); + + assertThatCountByQueryThrowsIAE(query, "Too many component keys in query"); + } + + @Test + public void countByQuery_throws_IAE_if_too_many_component_uuids() { + Set uuids = IntStream.range(0, 1_010).mapToObj(String::valueOf).collect(Collectors.toSet()); + ComponentQuery.Builder query = ComponentQuery.builder() + .setQualifiers(Qualifiers.PROJECT) + .setComponentUuids(uuids); + + assertThatCountByQueryThrowsIAE(query, "Too many component UUIDs in query"); + } + + private void assertThatCountByQueryThrowsIAE(ComponentQuery.Builder query, String expectedMessage) { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage(expectedMessage); + + underTest.countByQuery(dbSession, query.build()); + } + @Test public void select_ghost_projects() { OrganizationDto organization = db.organizations().insert(); @@ -1009,11 +1056,40 @@ public class ComponentDaoTest { } @Test - public void countByQuery_with_organization_throws_NPE_of_organizationUuid_is_null() { - expectedException.expect(NullPointerException.class); - expectedException.expectMessage("organizationUuid can't be null"); + public void selectByQuery_throws_IAE_if_too_many_component_ids() { + Set ids = LongStream.range(0L, 1_010L).boxed().collect(Collectors.toSet()); + ComponentQuery.Builder query = ComponentQuery.builder() + .setQualifiers(Qualifiers.PROJECT) + .setComponentIds(ids); - underTest.countByQuery(dbSession, null, ALL_PROJECTS_COMPONENT_QUERY); + assertThatSelectByQueryThrowsIAE(query, "Too many component ids in query"); + } + + @Test + public void selectByQuery_throws_IAE_if_too_many_component_keys() { + Set keys = IntStream.range(0, 1_010).mapToObj(String::valueOf).collect(Collectors.toSet()); + ComponentQuery.Builder query = ComponentQuery.builder() + .setQualifiers(Qualifiers.PROJECT) + .setComponentKeys(keys); + + assertThatSelectByQueryThrowsIAE(query, "Too many component keys in query"); + } + + @Test + public void selectByQuery_throws_IAE_if_too_many_component_uuids() { + Set uuids = IntStream.range(0, 1_010).mapToObj(String::valueOf).collect(Collectors.toSet()); + ComponentQuery.Builder query = ComponentQuery.builder() + .setQualifiers(Qualifiers.PROJECT) + .setComponentUuids(uuids); + + assertThatSelectByQueryThrowsIAE(query, "Too many component UUIDs in query"); + } + + private void assertThatSelectByQueryThrowsIAE(ComponentQuery.Builder query, String expectedMessage) { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage(expectedMessage); + + underTest.selectByQuery(dbSession, query.build(), 0, Integer.MAX_VALUE); } @Test -- 2.39.5