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.

Sign up for GitHub

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

Jump to bottom

repl: "dump" failing on case-sensitive file systems? #5229

Closed
srenatus opened this issue Oct 11, 2022 · 9 comments · Fixed by #5698
Closed

repl: "dump" failing on case-sensitive file systems? #5229

srenatus opened this issue Oct 11, 2022 · 9 comments · Fixed by #5698
Assignees
@Trolloldem
Labels
bug good first issue

Comments

@srenatus
Copy link
Contributor

That's the evidence: #5227 (comment)

There is no reason it should depend on case-insensitivity to work correctly. Let's make it work on case-sensitive FSes.
@stale
Copy link

stale bot commented Nov 10, 2022

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

@stale stale bot added the inactive label Nov 10, 2022
@burnerlee
Copy link
Contributor

@srenatus could I pick this up?

@stale stale bot removed the inactive label Dec 7, 2022
@burnerlee
Copy link
Contributor

@srenatus is converting the command to Lower case necessary here? Either we can remove this lower case conversion, or we would have to pass the filepath in this function to recover the case sensitive filepath.
To fix this error, the final command generated should have the original filepath (case-sensitive)

@srenatus
Copy link
Contributor Author

srenatus commented Dec 7, 2022

Thanks for looking into this. I don't know the reason for the lower-casing there; but I think we should do OK with splitting on space first, and lower-casing the first part only: that's the command. We'd leave the filepath argument untouched.

@srenatus
Copy link
Contributor Author

srenatus commented Dec 7, 2022

I've assigned you. Thanks for contributing 👏

@anderseknert
Copy link
Member

Any updates on this @burnerlee ?

@stale
Copy link

stale bot commented Feb 4, 2023

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

@stale stale bot added the inactive label Feb 4, 2023
@Trolloldem
Copy link
Contributor

Hi @srenatus. I see the issue is still present in OPA, may I try to finish what @burnerlee suggested and open a PR?

@stale stale bot removed the inactive label Feb 24, 2023
@srenatus srenatus assigned Trolloldem and unassigned burnerlee Feb 24, 2023
@srenatus
Copy link
Contributor Author

Sure!

Trolloldem added a commit to Trolloldem/opa that referenced this issue Feb 24, 2023
@Trolloldem
repl: only use ToLower on the input command
96176fd 
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 open-policy-agent#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: open-policy-agent#5229

Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
Trolloldem added a commit to Trolloldem/opa that referenced this issue Feb 24, 2023
@Trolloldem
repl: only use ToLower on the input command
91aba1f 
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 open-policy-agent#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: open-policy-agent#5229

Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
Trolloldem added a commit to Trolloldem/opa that referenced this issue Feb 24, 2023
@Trolloldem
repl: only use ToLower on the input command
9f700a7 
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 open-policy-agent#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: open-policy-agent#5229

Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
@Trolloldem Trolloldem mentioned this issue Feb 24, 2023
Merged
Trolloldem added a commit to Trolloldem/opa that referenced this issue Feb 24, 2023
@Trolloldem
repl: only use ToLower on the input command
6c9ece2 
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 open-policy-agent#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: open-policy-agent#5229
Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
anderseknert pushed a commit to Trolloldem/opa that referenced this issue Feb 27, 2023
@Trolloldem @anderseknert
repl: only use ToLower on the input command
4ff63ed 
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 open-policy-agent#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: open-policy-agent#5229
Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com>
anderseknert pushed a commit that referenced this issue Feb 27, 2023
@Trolloldem
repl: only use ToLower on the input command (#5698)
e4c7020 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Trolloldem Trolloldem

Labels
bug good first issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

repl: only use ToLower on the input command Trolloldem/opa
4 participants
@anderseknert @srenatus @Trolloldem @burnerlee