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

command CONFIG SET * reply array length error #2320

Closed
lqxhub opened this issue Jan 15, 2024 · 4 comments
Closed

command CONFIG SET * reply array length error #2320

lqxhub opened this issue Jan 15, 2024 · 4 comments
Labels
☢️ Bug Something isn't working

Comments

@lqxhub
Copy link
Collaborator

lqxhub commented Jan 15, 2024

Is this a regression?

Yes

Description

CONFIG SET *

 1) "timeout"
 2) "requirepass"
 3) "masterauth"
 4) "slotmigrate"
 5) "userpass"
 6) "userblacklist"
 7) "dump-prefix"
 8) "maxclients"
 9) "dump-expire"
10) "expire-logs-days"
.... 

Then execute any command

Such as:

get a

reply:

"write-buffer-size"

The reason is that the array length returned by the 'CONFIG SET *' command is incorrect

pika/src/pika_admin.cc

Lines 2065 to 2069 in ae5e8c6

void ConfigCmd::ConfigSet(std::string& ret, std::shared_ptr<Slot> slot) {
std::string set_item = config_args_v_[1];
if (set_item == "*") {
ret = "*29\r\n";
EncodeString(&ret, "timeout");

on 2068 line ret = "*29\r\n"; The number 29 was written down

Please provide a link to a minimal reproduction of the bug

No response

Screenshots or videos

image

Please provide the version you discovered this bug in (check about page for version information)

on unstable branch

Anything else?

No response

@lqxhub lqxhub added the ☢️ Bug Something isn't working label Jan 15, 2024
@578223592
Copy link
Contributor

看起来将回复的数字29改成一个更大的数字来包含所有回复就可以了?
我尝试一下

@lqxhub
Copy link
Collaborator Author

lqxhub commented Jan 15, 2024

看起来将回复的数字29改成一个更大的数字来包含所有回复就可以了? 我尝试一下

这样虽然可以, 但是不能一劳永逸, 我想用一个 vector吧, 先往 vector里塞,然后调用 AppendStringVector 返回吧

pika/include/pika_command.h

Lines 432 to 441 in ae5e8c6

void AppendStringVector(const std::vector<std::string>& strArray) {
if (strArray.empty()) {
AppendArrayLen(-1);
return;
}
AppendArrayLen(strArray.size());
for (const auto& item : strArray) {
AppendString(item);
}
}

这里也有个和redis不一样的地方,虽然客户端识别 还是一起改一下吧, 把 AppendArrayLen(-1); 需要改成 AppendArrayLen(0);

@578223592
Copy link
Contributor

578223592 commented Jan 15, 2024

看起来将回复的数字29改成一个更大的数字来包含所有回复就可以了? 我尝试一下

这样虽然可以, 但是不能一劳永逸, 我想用一个 vector吧, 先往 vector里塞,然后调用 AppendStringVector 返回吧

pika/include/pika_command.h

Lines 432 to 441 in ae5e8c6

void AppendStringVector(const std::vector<std::string>& strArray) {
if (strArray.empty()) {
AppendArrayLen(-1);
return;
}
AppendArrayLen(strArray.size());
for (const auto& item : strArray) {
AppendString(item);
}
}

这里也有个和redis不一样的地方,虽然客户端识别 还是一起改一下吧, 把 AppendArrayLen(-1); 需要改成 AppendArrayLen(0);

agree,我会一起改掉

@578223592
Copy link
Contributor

@lqxhub 尝试修改了,pr在: #2336

但是目前还有一些问题,在 #2336 中的4、5、6,希望得到解答,感谢!

AlexStocks pushed a commit that referenced this issue Jan 21, 2024
* fix

* change ConfigCmd::ConfigSet return way

* func ConfigCmd::ConfigSet use res_ to get return value
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this issue Jun 8, 2024
…#2320) (OpenAtomFoundation#2336)

* fix

* change ConfigCmd::ConfigSet return way

* func ConfigCmd::ConfigSet use res_ to get return value
cheniujh pushed a commit to cheniujh/pika that referenced this issue Sep 24, 2024
…#2320) (OpenAtomFoundation#2336)

* fix

* change ConfigCmd::ConfigSet return way

* func ConfigCmd::ConfigSet use res_ to get return value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☢️ Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants