This is an automated email from the ASF dual-hosted git repository.

git-hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git


The following commit(s) were added to refs/heads/unstable by this push:
     new b39fd25bd fix(config): reject invalid replicaof values that mix no/one 
keywords (#3663)
b39fd25bd is described below

commit b39fd25bdf783e31b49d8edad0f64f5eea7d0f52
Author: weimeng <[email protected]>
AuthorDate: Tue Oct 20 22:15:33 2026 +0800

    fix(config): reject invalid replicaof values that mix no/one keywords 
(#3663)
    
    ## Summary
    
    Fix the `replicaof` config callback to properly reject invalid values
    that
    partially match the "no one" keyword, such as "no 1234", "foo one", "noo
    one",
    or "no onee".
    
    ## Root Cause
    
    In `src/config/config.cc`, the `replicaof` callback used the condition:
    
    ```cpp
    if (args[0] != "no" && args[1] != "one") {
        // treat as host port
    } else if (args[0] == "no" && args[1] == "one") {
        // clear master
    }
    ```
    
    
    The && in the first condition means it only enters the host/port branch
    when
    neither arg is a keyword. Inputs like "no 1234" (host="no", port=1234)
    or
    "foo one" (host="foo", port parse fails with misleading error) would
    silently
    fall through to the host/port branch instead of being rejected.
    
    
    ## The Fix
    Add an explicit else if for the "no one" case and an else to reject any
    remaining invalid combination:
    
    ```cpp
    if (args[0] != "no" && args[1] != "one") {
        // valid host port
    } else if (args[0] == "no" && args[1] == "one") {
        master_host.clear();
        master_port = 0;
    } else {
        return {Status::NotOK, "invalid replicaof value, must be 'host port' or 
'no one'"};
    }
    ```
    
    
    ## AI Usage Disclosure
    - The bug fix was implemented manually.
    - AI tools(Qoder) were used to assist in generating unit test
    scaffolding; all tests were reviewed and verified by the author.
    
    ---------
    
    Co-authored-by: weimeng <[email protected]>
    Co-authored-by: Twice <[email protected]>
    Co-authored-by: hulk <[email protected]>
---
 .github/config/typos.toml                  |  3 ++
 src/config/config.cc                       |  2 +-
 tests/gocase/unit/config/replicaof_test.go | 61 ++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/.github/config/typos.toml b/.github/config/typos.toml
index 2b3be38f1..561a6f7a2 100644
--- a/.github/config/typos.toml
+++ b/.github/config/typos.toml
@@ -47,3 +47,6 @@ ignore-hidden = false
 
 #Fix name (Yann Collet)
 "Collet" = "Collet"
+
+#Used in replicaof test for typo in "no" keyword
+"noo" = "noo"
diff --git a/src/config/config.cc b/src/config/config.cc
index 11a69a025..d2f9aed25 100644
--- a/src/config/config.cc
+++ b/src/config/config.cc
@@ -543,7 +543,7 @@ void Config::initFieldCallback() {
     }
     std::vector<std::string> args = util::Split(v, " \t");
     if (args.size() != 2) return {Status::NotOK, "wrong number of arguments"};
-    if (args[0] != "no" && args[1] != "one") {
+    if (!util::EqualICase(args[0], "no") || !util::EqualICase(args[1], "one")) 
{
       master_host = args[0];
       auto parse_result = ParseInt<int>(args[1], NumericRange<int>{1, 
PORT_LIMIT - 1}, 10);
       if (!parse_result) {
diff --git a/tests/gocase/unit/config/replicaof_test.go 
b/tests/gocase/unit/config/replicaof_test.go
new file mode 100644
index 000000000..24b084edf
--- /dev/null
+++ b/tests/gocase/unit/config/replicaof_test.go
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package config
+
+import (
+       "context"
+       "testing"
+
+       "github.com/apache/kvrocks/tests/gocase/util"
+       "github.com/stretchr/testify/require"
+)
+
+func TestReplicaofNoOneLowerCase(t *testing.T) {
+       srv := util.StartServer(t, map[string]string{"replicaof": "no one"})
+       defer srv.Close()
+
+       rdb := srv.NewClient()
+       defer func() { require.NoError(t, rdb.Close()) }()
+
+       val := rdb.Info(context.Background(), "replication").Val()
+       require.Contains(t, val, "role:master")
+}
+
+func TestReplicaofNoOneUpperCase(t *testing.T) {
+       srv := util.StartServer(t, map[string]string{"replicaof": "NO ONE"})
+       defer srv.Close()
+
+       rdb := srv.NewClient()
+       defer func() { require.NoError(t, rdb.Close()) }()
+
+       val := rdb.Info(context.Background(), "replication").Val()
+       require.Contains(t, val, "role:master")
+}
+
+func TestReplicaofNoOneMixedCase(t *testing.T) {
+       srv := util.StartServer(t, map[string]string{"replicaof": "No One"})
+       defer srv.Close()
+
+       rdb := srv.NewClient()
+       defer func() { require.NoError(t, rdb.Close()) }()
+
+       val := rdb.Info(context.Background(), "replication").Val()
+       require.Contains(t, val, "role:master")
+}

Reply via email to