From e4c7020a34b63e5342f93b45206fece8adb79d9c Mon Sep 17 00:00:00 2001
From: Gianluca Oldani <oldanigianluca@gmail.com>
Date: Mon, 27 Feb 2023 13:04:35 +0100
Subject: [PATCH] repl: only use ToLower on the input command (#5698)

This commit changes the behavior of the function
`newCommand` in `repl.go`. The input of this
function is a string containing both a command
to be run and its arguments. Prior to this commit,
the function converted the entire input to lower
case, without any distinction between the command
and its arguments. This lead to the bug exposed
in issue #5229.
Both the lowercase command and all lower case
arguments are put as fields in the `command`
struct returned by the function.

After this commit, the only part of the string that
is converted to lower case is the command that is
being executed, while the provided arguments are
evaluated as provided to the function. The struct
returned now contains the lower
case command and the arguments as provided to the
function.

Fixes: #5229

Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
---
 repl/repl.go      |  5 +++--
 repl/repl_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/repl/repl.go b/repl/repl.go
index 39199bcf79..b7a82d7fa0 100644
--- a/repl/repl.go
+++ b/repl/repl.go
@@ -1354,12 +1354,13 @@ type command struct {
 }
 
 func newCommand(line string) *command {
-	p := strings.Fields(strings.TrimSpace(strings.ToLower(line)))
+	p := strings.Fields(strings.TrimSpace(line))
 	if len(p) == 0 {
 		return nil
 	}
+	inputCommand := strings.ToLower(p[0])
 	for _, c := range builtin {
-		if c.name == p[0] {
+		if c.name == inputCommand {
 			return &command{
 				op:   c.name,
 				args: p[1:],
diff --git a/repl/repl_test.go b/repl/repl_test.go
index 17824b7903..8b740b44fd 100644
--- a/repl/repl_test.go
+++ b/repl/repl_test.go
@@ -286,10 +286,6 @@ func TestDumpPath(t *testing.T) {
 	var buffer bytes.Buffer
 	repl := newRepl(store, &buffer)
 
-	// NOTE: We are converting the path to lowercase in repl.OneShot.
-	// In file systems that are case-sensitive, this test will fail if we use
-	// a CamelCase directory name.
-	// See: https://github.com/open-policy-agent/opa/pull/5227#issuecomment-1273975492
 	dir, err := os.MkdirTemp("", "dump-path-test")
 	if err != nil {
 		t.Fatal(err)
@@ -325,6 +321,53 @@ func TestDumpPath(t *testing.T) {
 	}
 }
 
+func TestDumpPathCaseSensitive(t *testing.T) {
+	ctx := context.Background()
+	input := `{"a": [1,2,3,4]}`
+	var data map[string]interface{}
+	err := util.UnmarshalJSON([]byte(input), &data)
+	if err != nil {
+		panic(err)
+	}
+	store := inmem.NewFromObject(data)
+	var buffer bytes.Buffer
+	repl := newRepl(store, &buffer)
+
+	dir, err := os.MkdirTemp("", "DumpPathCaseSensitiveTest")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Cleanup(func() {
+		err := os.RemoveAll(dir)
+		if err != nil {
+			t.Errorf("error cleaning up with RemoveAll(): %v", err)
+		}
+	})
+	file := filepath.Join(dir, "tmpfile")
+	if err := repl.OneShot(ctx, fmt.Sprintf("dump %s", file)); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	if buffer.String() != "" {
+		t.Errorf("Expected no output but got: %v", buffer.String())
+	}
+
+	bs, err := os.ReadFile(file)
+	if err != nil {
+		t.Fatalf("Expected file read to succeed but got: %v", err)
+	}
+
+	var result map[string]interface{}
+	if err := util.UnmarshalJSON(bs, &result); err != nil {
+		t.Fatalf("Expected json unmarshal to succeed but got: %v", err)
+	}
+
+	if !reflect.DeepEqual(data, result) {
+		t.Fatalf("Expected dumped json to equal %v but got: %v", data, result)
+	}
+}
+
 func TestHelp(t *testing.T) {
 	topics["deadbeef"] = topicDesc{
 		fn: func(w io.Writer) error {