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

INFO & CLUSTER commands failed when using RESP3 #802

Closed
carlosabalde opened this issue May 13, 2020 · 4 comments
Closed

INFO & CLUSTER commands failed when using RESP3 #802

carlosabalde opened this issue May 13, 2020 · 4 comments
Assignees
Labels

Comments

@carlosabalde
Copy link

The following toy implementation shows how execution of INFO command using current RESP3 support in master fails with error Protocol error, got "=" as reply type byte. This has been tested using Redis Server 6.0.1. Same logic using RESP2 works as expected.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>
#include <hiredis/hiredis.h>

#define HOST "127.0.0.1"
#define PORT 6400

static redisContext *connect(unsigned version)
{
    redisContext *ctx = redisConnect(HOST, PORT);
    assert(ctx != NULL);
    if (ctx->err) {
        printf("CONNECT: %s\n", ctx->errstr);
    }
    assert(ctx != NULL && !ctx->err);

    redisReply *reply = redisCommand(ctx, "HELLO %d", version);
    if (ctx->err) {
        printf("HELLO: %s\n", ctx->errstr);
    }
    assert(
        !ctx->err && reply != NULL &&
        ((version == 2 && reply->type == REDIS_REPLY_ARRAY) ||
         (version == 3 && reply->type == REDIS_REPLY_MAP)));
    freeReplyObject(reply);

    return ctx;
}

int
main(int argc, char *argv[])
{
    redisContext *ctx = connect(3);
    redisReply *reply = redisCommand(ctx, "INFO");
    if (ctx->err) {
        printf("INFO: %s\n", ctx->errstr);
    }
    assert(!ctx->err && reply != NULL);
    printf("%s\n", reply->str);
    freeReplyObject(reply);
}
@carlosabalde
Copy link
Author

Btw, I got same error with RESP3 + CLUSTER command.

@carlosabalde carlosabalde changed the title INFO command failed when using RESP3 INFO & CLUSTER commands failed when using RESP3 May 13, 2020
@michael-grunder michael-grunder self-assigned this May 13, 2020
michael-grunder added a commit to michael-grunder/hiredis that referenced this issue May 13, 2020
@michael-grunder
Copy link
Collaborator

michael-grunder commented May 13, 2020

Hi @carlosabalde, would you please try this branch on my fork.

Looks like verbatim string handling was just lost in the shuffle back and forth between Redis and hiredis.

@carlosabalde
Copy link
Author

Hi @michael-grunder,

I confirm your fork fixes both the issue shown in the snippet as well as other issues in a project I'm adding RESP3 support.

However now reply->type is REDIS_REPLY_STRING when using RESP2 but REDIS_REPLY_VERB when using RESP3. That's annoying when trying to support both protocols, but I'm assume that's correct, right? Any hint to understand when a command would return REDIS_REPLY_STRING both in RESP2 and RESP3 and when not?

Thanks a lot! :)

@michael-grunder
Copy link
Collaborator

Closing in favor of #805

michael-grunder added a commit that referenced this issue May 19, 2020
Pull RESP3 verbatim string handling from Redis

Fixes #802
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants