Comments (10)
Ja, I don't think setting/resetting of IFS is necessary. But you have to take care about (at least):
- the read builtin. Set IFS as keyword parameter in front of it. Or don't use read in a pipe at all. E.g.:
# stupid
printf -v n %\*s 125 ' ';
this_command=$(HISTTIMEFORMAT=$'\r'$n$'\r' builtin history 1);
- qouting of any function executions within the loops. If the user wants to use aliases or redefine/unset internal functions, there is also the next problem.
alias __bp_set_ret_value=ls;
__bp_set_ret_value;
So, I suggest:
if type -t "$precmd_function" 1>/dev/null; then
\__bp_set_ret_value "$__bp_last_ret_value" "$__bp_last_argument_prev_command"
"$precmd_function"
fi
One can even think about turning on the readonly attribute ...
from bash-preexec.
Ok sorry, I just realized the issue is specifically with IFS=_
, I misread _
as a placeholder for something non-standard. Using _
as the field separator seems like a Bad Idea™ in general, but it probably still makes sense to try to support this in bash-prexec.
I think the right first step is to add some unit tests that everything works as expected with a malicious IFS. Capturing and restoring the IFS may be the best option. We might be able to avoid it (e.g. by more aggressively quoting everything in bash-preexec), but that may be more trouble than it's worth.
from bash-preexec.
@gnachman Thanks for reporting! Did as you suggested storing it. I created a couple small helpers based on https://unix.stackexchange.com/a/264947. Give #65 a quick look when you get a sec. I'll cut a version as soon as it's merged.
from bash-preexec.
cc @d630 @dimo414 @lguelorget bash-preexec's ⭐️ contributors.
from bash-preexec.
I worry the proposed fix would essentially introduce the opposite problem, since the user's function would be invoked with bash-preexec's IFS
. Sure, we could do the same resetting/restoring logic inside the loops too, but that's pretty nasty.
What exactly is the behavior that's IFS-dependent currently? At a glance I don't see it.
from bash-preexec.
@dimo414 That's true. IFS will be set to bash-preexec's definition for precmd and preexec.
Maybe I'm over complicating things, but if I don't have default IFS our for loops will break. For read statements it's possible to set IFS on the same line. Wonder if I can do that with our loops as well.
from bash-preexec.
Updated to set original IFS across precmd
and preexec
functions
from bash-preexec.
if I don't have default IFS our for loops will break
Can you share a particular example? I believe that array-splitting (e.g. for e in "${my_array[@]}"
) is not impacted by IFS, though I certainly could be wrong about that. I wasn't able to break anything with a few attempts.
from bash-preexec.
Thanks for the feedback @dimo414 and @d630! Just submitted #66 to address by quoting functions and setting IFS for read as advised by @d630. Tested locally and added some unit tests, I believe this fixes the problem.
from bash-preexec.
@gnachman Just released 0.3.7 for the fix. Let me know if you still have any issues. Thanks for reporting and feedback!
from bash-preexec.
Related Issues (20)
- Preexec is called when exiting bash with Ctrl-D HOT 2
- Return immediately when running in Zsh HOT 3
- [Bug] HISTCONTROL ignore* setting should be respected HOT 7
- `__bp_preexec_invoke_exec "$_"` printed when running multiple commads
- Tests fail with bats 1.5.0 HOT 7
- official way to detect whether bash-preexec is loaded HOT 2
- Line number exit outpu command ? HOT 1
- Bash 5.1 supports making PROMPT_COMMAND an array HOT 5
- ShellCheck highlights several issues with the file - Shellcheck could be disabled, or issues resolved HOT 2
- post_cmd functionality addition HOT 2
- Functions called in opposite order in midnight commander subshell HOT 2
- Some tests fail with bats 1.9 HOT 6
- preexec() does not work where as precmd() is working as intended. HOT 1
- Library Authors documentation can still lead to issues HOT 2
- preexec $1 is missing some commands HOT 1
- detailed bash log with preexec and precmd HOT 2
- Not working on Windows using Bash in MSYS2 HOT 7
- preexec doesn't work for no-op command `:`
- `bash-preexec` will cause `hishtory` to be particularly time-consuming for commands including pipeline.
- New release when?
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from bash-preexec.