rubenada commented on code in PR #320:
URL: https://github.com/apache/calcite-avatica/pull/320#discussion_r3879096827
##########
server/src/test/java/org/apache/calcite/avatica/jdbc/JdbcMetaTest.java:
##########
@@ -202,6 +207,122 @@ public class JdbcMetaTest {
// Our opened connection should get closed when this race condition happens
Mockito.verify(conn2).close();
}
+
+ private static Set<String> setOf(String... names) {
+ return new HashSet<>(java.util.Arrays.asList(names));
+ }
+
+ @Test public void testCheckClientPropertiesNoRulesConfigured() {
+ // With both lists null, every client name is passed through.
+ JdbcMeta.checkClientProperties(null, null,
+ Collections.singletonMap("anything", "v"));
+ }
+
+ @Test public void testCheckClientPropertiesDenylistRejectsListedName() {
+ final ForbiddenConnectionPropertyException e =
+ assertThrows(ForbiddenConnectionPropertyException.class, () ->
+ JdbcMeta.checkClientProperties(setOf("propA", "propB"), null,
+ Collections.singletonMap("propA", "v")));
+ assertThat(e.getPropertyName(), is("propA"));
+ assertThat(e.getRule(),
is(ForbiddenConnectionPropertyException.Rule.DENYLIST));
+ assertThat(e.getMessage(), containsString("propA"));
+ assertThat(e.getMessage(), containsString("denylist"));
+ assertThat(e.getMessage(),
+ containsString(JdbcMeta.CLIENT_PROPERTIES_DENYLIST_KEY));
+ }
+
+ @Test public void testCheckClientPropertiesDenylistPermitsUnlistedName() {
+ // No exception when the name is not on the denylist.
+ JdbcMeta.checkClientProperties(setOf("propA"), null,
+ Collections.singletonMap("propOther", "v"));
+ }
+
+ @Test public void testCheckClientPropertiesAllowlistRejectsUnlistedName() {
+ final ForbiddenConnectionPropertyException e =
+ assertThrows(ForbiddenConnectionPropertyException.class, () ->
+ JdbcMeta.checkClientProperties(null, setOf("propA", "propB"),
+ Collections.singletonMap("propC", "v")));
+ assertThat(e.getPropertyName(), is("propC"));
+ assertThat(e.getRule(),
is(ForbiddenConnectionPropertyException.Rule.ALLOWLIST));
+ assertThat(e.getMessage(), containsString("propC"));
+ assertThat(e.getMessage(), containsString("allowlist"));
+ assertThat(e.getMessage(),
+ containsString(JdbcMeta.CLIENT_PROPERTIES_ALLOWLIST_KEY));
+ }
+
+ @Test public void testCheckClientPropertiesAllowlistPermitsListedName() {
+ JdbcMeta.checkClientProperties(null, setOf("propA", "propB"),
+ Collections.singletonMap("propA", "v"));
+ }
+
+ @Test public void testCheckClientPropertiesDenylistAppliedBeforeAllowlist() {
+ // A name on both lists is rejected by the denylist, not the allowlist,
+ // the denylist rule always wins because it is checked first.
+ final ForbiddenConnectionPropertyException e =
+ assertThrows(ForbiddenConnectionPropertyException.class, () ->
+ JdbcMeta.checkClientProperties(setOf("propA"), setOf("propA",
"propB"),
+ Collections.singletonMap("propA", "v")));
+ assertThat(e.getRule(),
is(ForbiddenConnectionPropertyException.Rule.DENYLIST));
+ }
+
+ @Test public void testCheckClientPropertiesEmptyMapBypassesChecks() {
+ // An empty client map trivially satisfies both lists; the check must
+ // not fabricate a rejection when there is nothing to check.
+ JdbcMeta.checkClientProperties(null, setOf("propA"),
Collections.emptyMap());
+ JdbcMeta.checkClientProperties(setOf("propA"), setOf("propA"),
+ Collections.emptyMap());
+ }
+
+ @Test public void testCheckClientPropertiesNullMapBypassesChecks() {
+ JdbcMeta.checkClientProperties(setOf("propA"), setOf("propA"), null);
+ }
+
+ @Test public void testParsePropertyNameListNullOrBlankIsUnconfigured() {
+ // A null or all-blank value means the rule was not configured; the
+ // parser must return null so the check treats it as absent, not as an
+ // empty allowlist that would reject everything.
Review Comment:
Typically some properties should always be needed (like user, password,
schema, etc).
The mechanism is based on backwards compatibility, so everything keeps
working as before (i.e. everything allowed by default). I guess if someone
would really want to reject all properties, it could be theoretically done by
defining one, single, "dummy" property on the allowlist, so de facto all real
properties will be rejected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
