Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address further CLIENT SETINFO suffix rules #3536

Merged
merged 2 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 61 additions & 9 deletions src/main/java/redis/clients/jedis/ClientSetInfoConfig.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,70 @@
package redis.clients.jedis;

/**
* This interface is to modify the behavior of internally executing CLIENT SETINFO command.
*/
public interface ClientSetInfoConfig {
import java.util.Arrays;
import java.util.HashSet;
import redis.clients.jedis.exceptions.JedisValidationException;

default boolean isDisabled() {
return false;
public final class ClientSetInfoConfig {

private final boolean disabled;

private final String libNameSuffix;

public ClientSetInfoConfig() {
this(false, null);
}

public ClientSetInfoConfig(boolean disabled) {
this(disabled, null);
}

/**
* @param libNameSuffix must not have braces ({@code ()[]{}}) and spaces will be replaced with hyphens
*/
public ClientSetInfoConfig(String libNameSuffix) {
this(false, libNameSuffix);
}

private ClientSetInfoConfig(boolean disabled, String libNameSuffix) {
this.disabled = disabled;
this.libNameSuffix = validateLibNameSuffix(libNameSuffix);
}

private static final HashSet<Character> BRACES = new HashSet<>(Arrays.asList('(', ')', '[', ']', '{', '}'));

private static String validateLibNameSuffix(String suffix) {
if (suffix == null || suffix.trim().isEmpty()) {
return null;
}

for (int i = 0; i < suffix.length(); i++) {
char c = suffix.charAt(i);
if (c < ' ' || c > '~' || BRACES.contains(c)) {
sazzad16 marked this conversation as resolved.
Show resolved Hide resolved
throw new JedisValidationException("lib-name suffix cannot contain braces, newlines or "
+ "special characters.");
}
}

return suffix.replaceAll("\\s", "-");
}

public final boolean isDisabled() {
return disabled;
}

public final String getLibNameSuffix() {
return libNameSuffix;
}

public static final ClientSetInfoConfig DEFAULT = new ClientSetInfoConfig();

public static final ClientSetInfoConfig DISABLED = new ClientSetInfoConfig(true);

/**
* If provided, this suffix will be enclosed by braces {@code ()}.
* @param suffix must not have braces ({@code ()[]{}}) and spaces will be replaced with hyphens
* @return config
*/
default String getLibNameSuffix() {
return null;
public static ClientSetInfoConfig withLibNameSuffix(String suffix) {
return new ClientSetInfoConfig(suffix);
}
}
4 changes: 2 additions & 2 deletions src/main/java/redis/clients/jedis/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -416,13 +416,13 @@ private void initializeFromClientConfig(final JedisClientConfig config) {
}

ClientSetInfoConfig setInfoConfig = config.getClientSetInfoConfig();
if (setInfoConfig == null) setInfoConfig = new ClientSetInfoConfig() { };
if (setInfoConfig == null) setInfoConfig = ClientSetInfoConfig.DEFAULT;

if (!setInfoConfig.isDisabled()) {
String libName = JedisMetaInfo.getArtifactId();
if (libName != null && validateClientInfo(libName)) {
String libNameSuffix = setInfoConfig.getLibNameSuffix();
if (libNameSuffix != null && validateClientInfo(libNameSuffix)) {
if (libNameSuffix != null) { // validation is moved into ClientSetInfoConfig constructor
libName = libName + '(' + libNameSuffix + ')';
}
fireAndForgetMsg.add(new CommandArguments(Command.CLIENT).add(Keyword.SETINFO)
Expand Down
62 changes: 0 additions & 62 deletions src/main/java/redis/clients/jedis/DefaultClientSetInfoConfig.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public static class Builder {

private HostAndPortMapper hostAndPortMapper = null;

private ClientSetInfoConfig clientSetInfoConfig = null;
private ClientSetInfoConfig clientSetInfoConfig = ClientSetInfoConfig.DEFAULT;

private Builder() {
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/redis/clients/jedis/JedisClientConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,6 @@ default HostAndPortMapper getHostAndPortMapper() {
* @return CLIENT SETINFO config
*/
default ClientSetInfoConfig getClientSetInfoConfig() {
return null;
return ClientSetInfoConfig.DEFAULT;
}
}
13 changes: 5 additions & 8 deletions src/test/java/redis/clients/jedis/JedisTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ public void checkDisconnectOnQuit() {
@Test
public void clientSetInfoDefault() {
try (Jedis jedis = new Jedis(hnp, DefaultJedisClientConfig.builder().password("foobared")
.build())) {
.clientSetInfoConfig(ClientSetInfoConfig.DEFAULT).build())) {
assertEquals("PONG", jedis.ping());
String info = jedis.clientInfo();
assertTrue(info.contains("lib-name=" + JedisMetaInfo.getArtifactId()));
Expand All @@ -301,11 +301,9 @@ public void clientSetInfoDefault() {
}

@Test
public void clientSetInfoDisable() {
public void clientSetInfoDisabled() {
try (Jedis jedis = new Jedis(hnp, DefaultJedisClientConfig.builder().password("foobared")
.clientSetInfoConfig(new ClientSetInfoConfig() {
@Override public boolean isDisabled() { return true; }
}).build())) {
.clientSetInfoConfig(ClientSetInfoConfig.DISABLED).build())) {
assertEquals("PONG", jedis.ping());
String info = jedis.clientInfo();
assertFalse(info.contains("lib-name=" + JedisMetaInfo.getArtifactId()));
Expand All @@ -314,10 +312,9 @@ public void clientSetInfoDisable() {
}

@Test
public void clientSetInfoCustom() {
public void clientSetInfoLibNameSuffix() {
final String libNameSuffix = "for-redis";
ClientSetInfoConfig setInfoConfig = DefaultClientSetInfoConfig.builder()
.libNameSuffix(libNameSuffix).build();
ClientSetInfoConfig setInfoConfig = ClientSetInfoConfig.withLibNameSuffix(libNameSuffix);
try (Jedis jedis = new Jedis(hnp, DefaultJedisClientConfig.builder().password("foobared")
.clientSetInfoConfig(setInfoConfig).build())) {
assertEquals("PONG", jedis.ping());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package redis.clients.jedis.misc;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;

import java.util.Arrays;
import org.junit.Test;

import redis.clients.jedis.ClientSetInfoConfig;
import redis.clients.jedis.exceptions.JedisValidationException;

public class ClientSetInfoConfigTest {

@Test
public void replaceSpacesWithHyphens() {
assertEquals("Redis-Java-client",
ClientSetInfoConfig.withLibNameSuffix("Redis Java client").getLibNameSuffix());
}

@Test
public void errorForBraces() {
Arrays.asList('(', ')', '[', ']', '{', '}')
.forEach(brace -> assertThrows(JedisValidationException.class,
() -> ClientSetInfoConfig.withLibNameSuffix("" + brace)));
}
}
Loading