Skip to content

Commit

Permalink
Merge pull request #157 from jasonk000/jkoch/fast-key-validator
Browse files Browse the repository at this point in the history
perf: Avoid charset lookup in key validation
  • Loading branch information
pkarumanchi9 authored Feb 5, 2025
2 parents 962b76b + 168aa2b commit 4cb82c2
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package com.netflix.evcache.pool;

import java.nio.charset.StandardCharsets;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;

import com.netflix.evcache.EVCacheKey;
import net.spy.memcached.MemcachedClientIF;
import net.spy.memcached.transcoders.Transcoder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -20,6 +22,24 @@ public class EVCacheClientUtil {
private final String _appName;
private final long _operationTimeout;

/**
* Maximum supported key length.
*/
private static final int MAX_KEY_BYTES = MemcachedClientIF.MAX_KEY_LENGTH;

/**
* Exception thrown if the input key is too long.
*/
private static final IllegalArgumentException KEY_TOO_LONG_EXCEPTION =
new IllegalArgumentException("Key is too long (maxlen = "
+ MAX_KEY_BYTES + ')');

/**
* Exception thrown if the input key is empty.
*/
private static final IllegalArgumentException KEY_EMPTY_EXCEPTION =
new IllegalArgumentException("Key must contain at least one character.");

public EVCacheClientUtil(String appName, long operationTimeout) {
this._appName = appName;
this._operationTimeout = operationTimeout;
Expand Down Expand Up @@ -85,4 +105,37 @@ private EVCacheLatch fixup(EVCacheClient sourceClient, EVCacheClient[] destClien
}
return latch;
}

/**
* Check if a given key is valid to transmit.
*
* @param key the key to check.
* @param binary if binary protocol is used.
*/
public static void validateKey(final String key, final boolean binary) {
// This is a replica of the upstream StringUtils.validateKey from the
// memcached implementation, however we modify it to avoid the Charset
// lookup cost, which ends up the majority of the validation.

byte[] keyBytes = key.getBytes(StandardCharsets.UTF_8);
int keyLength = keyBytes.length;

if (keyLength > MAX_KEY_BYTES) {
throw KEY_TOO_LONG_EXCEPTION;
}

if (keyLength == 0) {
throw KEY_EMPTY_EXCEPTION;
}

if(!binary) {
for (byte b : keyBytes) {
if (b == ' ' || b == '\n' || b == '\r' || b == 0) {
throw new IllegalArgumentException(
"Key contains invalid characters: ``" + key + "''");
}
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiPredicate;

import com.netflix.evcache.pool.EVCacheClientUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -208,7 +209,7 @@ public <T> EVCacheBulkGetFuture<T> asyncGetBulk(Collection<String> keys,

//Populate Node and key Map
for (String key : keys) {
StringUtils.validateKey(key, opFact instanceof BinaryOperationFactory);
EVCacheClientUtil.validateKey(key, opFact instanceof BinaryOperationFactory);
final MemcachedNode primaryNode = locator.getPrimary(key);
if (primaryNode.isActive() && nodeValidator.test(primaryNode, key)) {
Collection<String> ks = chunks.computeIfAbsent(primaryNode, k -> new ArrayList<>());
Expand Down

0 comments on commit 4cb82c2

Please sign in to comment.