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.

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

main: Fix alias loops in nested lists (e.g. command substitution) #796

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

phy1729
Copy link
Member

@phy1729 phy1729 commented Feb 25, 2021

alias ls='echo $(ls)' then ls will cause zsh to crash. This avoids that.

The idea is that we are still in the alias even when recursing into a nested list like a command substitution, so _highlight_list is the wrong place to local seen_alias.


BUFFER='ls'

expected_region_highlight=(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention that this test will crash zsh (infinite recursion, presumably?) if the bug being tested for is present?

@danielshahaf
Copy link
Member

The idea is that we are still in the alias even when recursing into a nested list like a command substitution, so _highlight_list is the wrong place to local seen_alias.

What about the opposite problem? Might some $seen_alias element remain set longer than it should?

@phy1729
Copy link
Member Author

phy1729 commented Mar 1, 2021

I can't imagine a scenario in which an alias is being expanded and in the course of expansion it is again eligible for expansion. That way lies madness^Wstack overflows.

@danielshahaf
Copy link
Member

The C code enforces that an alias be ineligible for expansion whilst it's being expanded. It's easy to see that: alias ls=ls works.

@danielshahaf
Copy link
Member

I don't immediately follow how that's the same thing as what I asked, though. I'll look again later.

@phy1729
Copy link
Member Author

phy1729 commented Mar 2, 2021

Seems alias ls='eval ls' then ls will cause zsh to segfault. Of course it's possible to make such a construct not segfault; however, both with and without the patch z-sy-h will highlight the alias as such since we don't peek into eval.

The point being the only way for seen_alias to be set longer than it ought is if an alias is eligible for expansion while it is itself being expanded. That seems to only be the case with eval.

@danielshahaf
Copy link
Member

That answers my question, then. I don't have any other review comments.

Would you report the segfault, please? Either upstream or here, whichever's appropriate. Thanks!

@danielshahaf
Copy link
Member

danielshahaf commented Mar 2, 2021 via email

@danielshahaf
Copy link
Member

danielshahaf commented Mar 2, 2021 via email

@danielshahaf
Copy link
Member

Should this be milestoned 0.8.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants