Skip to content

Memory corruption on large responses for ft.search #3526

@garry-mcfly

Description

@garry-mcfly

Bug Report

When using ftSearch and the response from Redis is large (~200KB), it results in memory corruption.
Depending on the netty version, this either crashes the JVM process or it simply produces garbage in the returned results.
It seems that ByteBuffer from data read at the beginning of the processing is overriden by later data.

Current Behavior

  • netty 4.2.4.Final
    • JVM crashes in thread "lettuce-nioEventLoop" in sub-routine jshort_disjoint_arraycopy
  • netty 4.1.118.Final
    • on protocol version RESP2 the result is just garbage
    • on protocol version RESP3 the result is just empty (because the SearchReplyParser can not find "results" key in the garbage ByteBuffers)

Input Code

The following code can be used to reproduce the behaviour:

Input Code
package io.lettuce.core;

import io.lettuce.core.api.StatefulRedisConnection;
import io.lettuce.core.json.JsonPath;
import io.lettuce.core.protocol.DecodeBufferPolicies;
import io.lettuce.core.protocol.ProtocolVersion;
import io.lettuce.core.search.arguments.CreateArgs;
import io.lettuce.core.search.arguments.NumericFieldArgs;
import io.lettuce.core.search.arguments.SearchArgs;
import io.lettuce.core.search.arguments.SortByArgs;
import org.opentest4j.AssertionFailedError;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class RediSearchBugProver {

    private static final Logger log = LoggerFactory.getLogger(RediSearchBugProver.class);

    public static void main(String[] args) {
        // System.setProperty("org.slf4j.simpleLogger.log.io.lettuce.core.protocol", "trace");
        var uri = RedisURI.create("localhost", 6379);
        try (var client = RedisClient.create(uri)) {
            client.setOptions(ClientOptions.builder()
                    // increase it drastically just to be sure discardBytesRatio is not the cause
                    .decodeBufferPolicy(DecodeBufferPolicies.ratio(Integer.MAX_VALUE / 2.0f))
                    // ft.search result on RESP2 is an array, on RESP3 a map. So error is different
                    .protocolVersion(ProtocolVersion.RESP2)
                    .build());
            try (var connection = client.connect()) {
                runTest(connection, String.valueOf(System.currentTimeMillis()));
            }
        }
    }

    private static void runTest(StatefulRedisConnection<String, String> con, String space) {
        var prefix = "test-" + space + ":";
        var index = "idx-" + space;

        log.info("===> running with {}, {}", prefix, index);
        con.sync().ftCreate(
                index,
                CreateArgs.<String, String>builder()
                        .on(CreateArgs.TargetType.JSON)
                        .withPrefix(prefix)
                        .build(),
                List.of(NumericFieldArgs.<String>builder().name("pos").build())
        );
        var searchArgs = SearchArgs.<String, String>builder()
                .sortBy(SortByArgs.<String>builder().attribute("pos").build())
                .limit(0, 10_000)
                .build();

        var expected = new ArrayList<>();
        for (int i = 1; i <= 1000; i++) {
            var latest = """
                    {"pos":%d,"ts":%d,"large":"just here to make the response larger to some great extend and overflow the buffers"}
                    """.formatted(i, System.currentTimeMillis()).trim();
            // lettuce<7.x: con.sync().jsonSet(prefix + i, JsonPath.ROOT_PATH, con.sync().getJsonParser().createJsonValue(latest));
            con.sync().jsonSet(prefix + i, JsonPath.ROOT_PATH, latest);
            expected.add(latest);

            if (i >= 924) {
                log.info("=== search {}", i);
                var searchReply = con.sync().ftSearch(index, "*", searchArgs);
                // with RESP3 this simply returns 0 as the map keys in Resp3SearchResultsParser are not found
                assertEquals(expected.size(), searchReply.getCount());
                for (int t = 1; t <= expected.size(); t++) {
                    var fields = searchReply.getResults().get(t - 1).getFields();
                    try {
                        assertEquals(expected.get(t - 1), fields.get("$"));
                    } catch (AssertionFailedError e) {
                        // with RESP2 this shows strange fields instead of the expected '$={"pos":..."'
                        log.info("Fields at pos {}: {}", t - 1, fields);
                        throw new AssertionFailedError("On loop " + i + ": " + e.getMessage(), e);
                    }
                }
            }
        }
    }

}
pom.xml
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>io.lettuce.core</groupId>
    <artifactId>lettuce-redis-search-bug</artifactId>
    <version>1.0-SNAPSHOT</version>

    <properties>
        <maven.compiler.source>21</maven.compiler.source>
        <maven.compiler.target>21</maven.compiler.target>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>

        <lettuce-core.version>7.1.0.RELEASE</lettuce-core.version>
        <!-- with 4.1 it reports wrong data -->
        <netty.version>4.1.118.Final</netty.version>
        <!-- with 4.2 it crashes the jvm -->
        <!--netty.version>4.2.4.Final</netty.version-->
    </properties>

    <dependencyManagement>
        <dependencies>
            <dependency>
                <groupId>io.netty</groupId>
                <artifactId>netty-bom</artifactId>
                <version>${netty.version}</version>
                <type>pom</type>
                <scope>import</scope>
            </dependency>
        </dependencies>
    </dependencyManagement>

    <dependencies>
        <dependency>
            <groupId>io.lettuce</groupId>
            <artifactId>lettuce-core</artifactId>
            <version>${lettuce-core.version}</version>
        </dependency>
        <dependency>
            <groupId>io.netty</groupId>
            <artifactId>netty-common</artifactId>
        </dependency>
        <dependency>
            <groupId>io.netty</groupId>
            <artifactId>netty-handler</artifactId>
        </dependency>
        <dependency>
            <groupId>io.netty</groupId>
            <artifactId>netty-transport</artifactId>
        </dependency>

        <dependency>
            <groupId>org.projectlombok</groupId>
            <artifactId>lombok</artifactId>
            <version>1.18.42</version>
        </dependency>
        <dependency>
            <groupId>org.junit.jupiter</groupId>
            <artifactId>junit-jupiter-api</artifactId>
            <version>6.0.1</version>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-simple</artifactId>
            <version>2.0.17</version>
        </dependency>
    </dependencies>

</project>

Expected behavior/code

The ftSearch() function correctly returns the results.

Environment

  • Lettuce version(s): 7.1.0.RELEASE, 6.8.1.RELEASE
  • Redis version: redis/redis-stack-server:7.2.0-v10 (Redis 7.2.4)
  • Java: Amazon Corretto 21.0.1
  • Netty versions: 4.1.118.Final, 4.2.4.Final

Additional context

Some notes to checks already done:

  • the default DecodeBufferPolicy (with buffer.discardReadBytes()) is not the cause of the error
  • when debugging io.lettuce.core.protocol.CommandHandler#channelRead
    • parsing of the response is working fine
    • the values put into ComplexOutput are correct when being put but later becoming garbage
    • the readBuffer contains the correct data when the ft-search command is completed
    • so it may have todo with the automatic expansion during readBuffer.writeBytes(input)
  • the old implementation in lettucemod's SearchOutput created the documents on the fly and therefore was not affected
    • the new implemenation using ComplexOutput keeps all the ByteBuffers and later transforms them in SearchReplyParser
  • I tried replacing buffer.internalNioBuffer by buffer.nioBuffer in RedisStateMachine#readBytes0 but it has no effect

Metadata

Metadata

Assignees

No one assigned

    Labels

    for: team-attentionAn issue we need to discuss as a team to make progressstatus: mre-availableMinimal Reproducible Example is availabletype: bugA general bug

    Type

    No fields configured for Bug.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions