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

NullPointerException if INFO command on redis cluster fails #2243

Closed
m-ibot opened this issue Nov 1, 2022 · 2 comments
Closed

NullPointerException if INFO command on redis cluster fails #2243

m-ibot opened this issue Nov 1, 2022 · 2 comments
Labels
type: bug A general bug
Milestone

Comments

@m-ibot
Copy link

m-ibot commented Nov 1, 2022

Bug Report

Current Behavior

A NullPointerException occours in lettuce core version 6.1.10.RELEASE if the INFO-Command on a Redis cluster fails. I assume the issue will also affect 6.2.1.RELEASE, but I did not test this. We ran into this Problem when updating our application to Spring Boot 2.6.13, which updated the lettuce-core dependency.

Nearly the same issue was already reported (and fixed) with #2035, but the error appears again after the changes made for #2161.

If a Redis cluster is used, lettuce-core will send an INFO command to the cluster. See NodeTopologyView line 103 and ongoing

    static NodeTopologyView from(RedisURI redisURI, Requests clusterNodesRequests, Requests infoRequests) {

        TimedAsyncCommand<String, String, String> nodes = clusterNodesRequests.getRequest(redisURI);
        TimedAsyncCommand<String, String, String> info = infoRequests.getRequest(redisURI);               // <- INFO request is send

        if (resultAvailable(nodes) && !nodes.isCompletedExceptionally() && resultAvailable(info)) {
            return new NodeTopologyView(redisURI, nodes.join(), optionallyGet(info), nodes.duration());
        }
        return new NodeTopologyView(redisURI);
    }

    private static <T> T optionallyGet(TimedAsyncCommand<?, ?, T> command) {

        if (command.isCompletedExceptionally()) { 
            return null;                                                                                  // <- null is returned
        }
        return command.join();
    }

When the client count is supposed to be extracted from the INFOresult, this null value is later passed as the haystack variable to Pattern#matcher(java.lang.CharSequence), which can not handle null values. See NodeTopologyView line 84 and ongoing

    private int getClientCount(String info) {
        return getMatchOrDefault(info, CONNECTED_CLIENTS_PATTERN, Integer::parseInt, 0); // <- client count is extracted from the INFO result, but 0 seems to be a valid default value
    }
    
    // ...
    
    private static <T> T getMatchOrDefault(String haystack, Pattern pattern, Function<String, T> converter, T defaultValue) {

        Matcher matcher = pattern.matcher(haystack);                                     // <- this line will result in a NullPointerException
        // ...
    }

See the Stack-Trace for details:

Stack trace
org.springframework.data.redis.RedisConnectionFailureException: Unable to connect to Redis; nested exception is java.lang.NullPointerException: Cannot invoke "java.lang.CharSequence.length()" because "this.text" is null
    at org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory$ExceptionTranslatingConnectionProvider.translateException(LettuceConnectionFactory.java:1689)
    at org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory$ExceptionTranslatingConnectionProvider.getConnection(LettuceConnectionFactory.java:1597)
    at org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory$SharedConnection.getNativeConnection(LettuceConnectionFactory.java:1383)
    at org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory$SharedConnection.getConnection(LettuceConnectionFactory.java:1366)
    at org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory.getSharedClusterConnection(LettuceConnectionFactory.java:1106)
    at org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory.getClusterConnection(LettuceConnectionFactory.java:441)
    at org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory.getConnection(LettuceConnectionFactory.java:417)
    // ...
    at org.springframework.cloud.sleuth.instrument.async.TraceRunnable.run(TraceRunnable.java:64)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
    at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.NullPointerException: Cannot invoke "java.lang.CharSequence.length()" because "this.text" is null
    at java.base/java.util.regex.Matcher.getTextLength(Matcher.java:1769)
    at java.base/java.util.regex.Matcher.reset(Matcher.java:415)
    at java.base/java.util.regex.Matcher.<init>(Matcher.java:252)
    at java.base/java.util.regex.Pattern.matcher(Pattern.java:1134)
    at io.lettuce.core.cluster.topology.NodeTopologyView.getMatchOrDefault(NodeTopologyView.java:94)
    at io.lettuce.core.cluster.topology.NodeTopologyView.getClientCount(NodeTopologyView.java:85)
    at io.lettuce.core.cluster.topology.NodeTopologyView.<init>(NodeTopologyView.java:73)
    at io.lettuce.core.cluster.topology.NodeTopologyView.from(NodeTopologyView.java:109)
    at io.lettuce.core.cluster.topology.DefaultClusterTopologyRefresh.getNodeSpecificViews(DefaultClusterTopologyRefresh.java:229)
    at io.lettuce.core.cluster.topology.DefaultClusterTopologyRefresh.lambda$null$1(DefaultClusterTopologyRefresh.java:108)
    at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:646)
    at java.base/java.util.concurrent.CompletableFuture$Completion.run(CompletableFuture.java:482)
    at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174)
    at io.netty.util.concurrent.DefaultEventExecutor.run(DefaultEventExecutor.java:66)
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    ... 1 more

Expected behavior:

A useful exception is thrown or (if possible) the the failed INFO command is handled in a way, that the further execution is possible without any error.

Environment

  • Lettuce version(s): 6.1.10.RELEASE
  • Redis version: 6.2
  • Java version: 11 and 17

Possible Solution

I'm not sure, if the result of INFO command is absolutely required.

If yes, the code should handle this case. Instead of returning null in the optionallyGet method in the case of command.isCompletedExceptionally() an exception should be thrown that gives usefull feedback and developers can handle the exception if needed or at least pin down the cause of the error.

If the INFO result is not required, optionallyGet should return Optional<T> instead of a T. In case of an error Optional.empty() should be returned. This could help to avoid NullPointerExceptions in the future, so that no null values are passed to any method that doesn't support it. I think this might be a solution since the getClientCount method has a possible default value of 0.

Since a NullPointerException was fixed in #2035 already, I suggest to avoid handling possible null values here and use Optional instead.

Additional context

There are several reasons, why the INFO request might fail. In my case the reason is not a network issue or similar. On our Redis cluster all Redis commands, that are marked as @dangerous, are blocked by an ACL rule. And the INFO command is one of these commands. I think, blocking commands of a category like @dangerous is not an uncommon scenario, it is even described in the Redis ACL documentation.

This behavior could also be reproduced by adding rename-command INFO "" to the redis config. This might be easier to reproduce this issue than handling ACL's

As a workaround we configured a temporary exception for the INFO command on our redis cluster.

@mp911de mp911de added the type: bug A general bug label Nov 8, 2022
@mp911de mp911de added this to the 6.2.2.RELEASE milestone Nov 14, 2022
@mp911de
Copy link
Collaborator

mp911de commented Nov 22, 2022

That's fixed now.

@mp911de mp911de closed this as completed Nov 22, 2022
@m-ibot
Copy link
Author

m-ibot commented Nov 22, 2022

Hi, thank you for fixing this bug! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants