[FIX] Fix quote removal coming from variable expansions#177
Conversation
244fd4f to
fe9f8c7
Compare
There was a problem hiding this comment.
With the new expander framework, the core functions that do the actual expansion got very simple.
All the data they need for the string replacement is already saved in the expander_task struct.
There was a problem hiding this comment.
Same here, no more logic required.
There was a problem hiding this comment.
The only logic left here is that if a variable was not found in the environment, it still has to replace the $VARNAME.
This is done by giving an empty string "" as the replacement argument to ft_strrplc_part().
| return (-1); | ||
| } | ||
|
|
||
| bool is_open_pair(unsigned char c, t_is_open_pair_op operation) |
There was a problem hiding this comment.
Just moved from expander_utils.c to string_utils.c
| !execute_expander_task_stack(new_str, task_stack, lst, shell)) | ||
| return (ft_lstclear(&task_stack, (void *)free_expander_task), false); | ||
| if (is_null_expansion(*new_str, task_stack)) | ||
| ft_free_and_null((void **)new_str); |
There was a problem hiding this comment.
If nothing but an empty string "" is left after expansion (like in f.e. $NOVAR), make it NULL. Unless it was previously in quotes.
The is_null_expansion() function goes through the whole task_stack and checks for that, i.e. if there was any quote removal performed.
| } | ||
|
|
||
| bool execute_expander_task_stack(char **new_str, t_list *task_stack, | ||
| t_list **lst, t_shell *shell) |
There was a problem hiding this comment.
Modifies the new_str at the saved indexes provided by the task_stack. The characters not relevant for the expander stay untouched.
| size_t start; | ||
|
|
||
| if (is_open_pair('\'', OP_GET)) | ||
| return (false); |
There was a problem hiding this comment.
If this function gets called and it is already inside of a single quote, it returns false to indicate that the end of a single quote pair has is already here and that the index has not been moved.
This has to be done bc there can be no nested single quote pairs.
With double quotes, nested pairs are possible:
"${"VAR"}"
The quotes inside the braces are a pair, as well as the quotes outside.
| size_t i; | ||
|
|
||
| if (!(op_mask & (E_EXPAND | E_HEREDOC))) | ||
| return (false); |
There was a problem hiding this comment.
If no variable expansion shall be performed, bad substitution is not possible and should therefor not be checked.
cee3370 to
48400bc
Compare
Instead of immediatly expanding into the string, create a stack of tasks that mark the index and the length of one section important for the expander. A stack, bc the expander has to finally expand the sequences from the back of the string to the front. This way, the indexes don't have to be updated. The removal of quotes coming from variables issue gets fixed bc at time of creation of the tasks, the string to be expanded does not get changed yet, and so the contents of the variables cannot disrupt. * Resolves [BUG] Quotes that came from variable expansions should not be removed #173
(Good to know the memory leak test works 😊)
Test case: `export B='${'`
48400bc to
c5bcc75
Compare
|
For me as a project developer and the corner case is not clearly declared in the subject so we have the right to design our own shell behavior. Doing the evaluation we have a very clearly defense why do we decide to design in these way with a make sense reason that is totally enough. |
|
In my mind, we are not going to be trained as a coding monkey but trained ourselves to have a higher view like a researcher and designer is much more important |
BUG: This breaks the behavior of quotes that come from variable expansions, as was already solved in PR #177. - Add wildcard tasks to task list to be able to differentiate which asterisks should be expanded and which ones not bc they were quoted. - Append quote tasks only after parameter expansion together with wildcard tasks. This is so that the wildcard tasks can be appended to the task list in correct order with the quote tasks. Otherwise, it would cause issues with the current implementation of how updating the task_list works after an expansion. - Also fix norminette for wildcard expansion and separate new functions into other files.
BUG: This breaks the behavior of quotes that come from variable expansions, as was already solved in PR #177. - Add wildcard tasks to task list to be able to differentiate which asterisks should be expanded and which ones not bc they were quoted. - Append quote tasks only after parameter expansion together with wildcard tasks. This is so that the wildcard tasks can be appended to the task list in correct order with the quote tasks. Otherwise, it would cause issues with the current implementation of how updating the task_list works after an expansion. - Also fix norminette for wildcard expansion and separate new functions into other files.
BUG: This breaks the behavior of quotes that come from variable expansions, as was already solved in PR #177. - Add wildcard tasks to task list to be able to differentiate which asterisks should be expanded and which ones not bc they were quoted. - Append quote tasks only after parameter expansion together with wildcard tasks. This is so that the wildcard tasks can be appended to the task list in correct order with the quote tasks. Otherwise, it would cause issues with the current implementation of how updating the task_list works after an expansion. - Also fix norminette for wildcard expansion and separate new functions into other files.
BUG: This breaks the behavior of quotes that come from variable expansions, as was already solved in PR #177. - Add wildcard tasks to task list to be able to differentiate which asterisks should be expanded and which ones not bc they were quoted. - Append quote tasks only after parameter expansion together with wildcard tasks. This is so that the wildcard tasks can be appended to the task list in correct order with the quote tasks. Otherwise, it would cause issues with the current implementation of how updating the task_list works after an expansion. - Also fix norminette for wildcard expansion and separate new functions into other files.
BUG: This breaks the behavior of quotes that come from variable expansions, as was already solved in PR #177. - Add wildcard tasks to task list to be able to differentiate which asterisks should be expanded and which ones not bc they were quoted. - Append quote tasks only after parameter expansion together with wildcard tasks. This is so that the wildcard tasks can be appended to the task list in correct order with the quote tasks. Otherwise, it would cause issues with the current implementation of how updating the task_list works after an expansion. - Also fix norminette for wildcard expansion and separate new functions into other files.
BUG: This breaks the behavior of quotes that come from variable expansions, as was already solved in PR #177. - Add wildcard tasks to task list to be able to differentiate which asterisks should be expanded and which ones not bc they were quoted. - Append quote tasks only after parameter expansion together with wildcard tasks. This is so that the wildcard tasks can be appended to the task list in correct order with the quote tasks. Otherwise, it would cause issues with the current implementation of how updating the task_list works after an expansion. - Also fix norminette for wildcard expansion and separate new functions into other files.
Instead of immediatly expanding into the string, create a stack of tasks that mark the index and the length of one section important for the expander.
A stack, bc the expander has to finally expand the sequences from the back of the string to the front.
This way, the indexes don't have to be updated after one section of the string has been expanded.
The issue of the removal of quotes coming from variables gets fixed bc at time of creation of the tasks, the string to be expanded does not get changed yet, and so the contents of the variables cannot disrupt.
Why the regression test fails:
$"<string>"(locale translation) and$'<string>'(escape sequence) ...... have a special meaning in bash, that's why the dollar sign and the quotes get removed.
Previously I just removed the
$, but I didn't fully understand why it should be this way.Now that I know, I decided to not do that and handle the
$like every other character in such a case, and therefore keep the dollar sign, bc we don't support these features (locale translation and escape sequence).I manually tested all failed test cases of the tester, and in all cases the only difference to bash is that we don't remove the dollar sign.
Bash manual references:
Dollar single quote
Dollar double quote