Bug 1425326 - gluster bash completion leaks TOP=0 into the environment
Summary: gluster bash completion leaks TOP=0 into the environment
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: scripts   
(Show other bugs)
Version: 4.1
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: bugs@gluster.org
QA Contact:
URL:
Whiteboard:
Keywords: Reopened, Triaged
Depends On: 1425325
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-02-21 08:39 UTC by Niels de Vos
Modified: 2018-08-29 12:44 UTC (History)
10 users (show)

Fixed In Version: glusterfs-4.1.3
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: 1425325
Environment:
Last Closed: 2018-08-29 12:44:28 UTC
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

Description Niels de Vos 2017-02-21 08:39:35 UTC
+++ This bug was initially created as a clone of Bug #1425325 +++

+++ This bug was initially created as a clone of Bug #1360888 +++

Description of problem:

On my machine I have a bash shell variable called "TOP" defined:

$ echo $TOP
0

(Note this is not an environment variable).  This breaks
various things, in this case the lowRISC initialization script.

The variable comes from gluster bash completion script
/etc/bash_completion.d/gluster

Version-Release number of selected component (if applicable):

glusterfs-cli-3.8.0-0.2.rc2.fc24.x86_64

How reproducible:

100%

Steps to Reproduce:
1. echo $TOP

--- Additional comment from Niels de Vos on 2016-09-12 07:37:16 CEST ---

All 3.8.x bugs are now reported against version 3.8 (without .x). For more information, see http://www.gluster.org/pipermail/gluster-devel/2016-September/050859.html

--- Additional comment from Anoop C S on 2017-02-21 07:16:52 CET ---

(In reply to Richard W.M. Jones from comment #0)
> Description of problem:
> 
> On my machine I have a bash shell variable called "TOP" defined:
> 
> $ echo $TOP
> 0
> 
> (Note this is not an environment variable).  This breaks
> various things, in this case the lowRISC initialization script.
> 

Can you please direct towards this initialization script?

And how severe is the issue?

> The variable comes from gluster bash completion script
> /etc/bash_completion.d/gluster
> 
> Version-Release number of selected component (if applicable):
> 
> glusterfs-cli-3.8.0-0.2.rc2.fc24.x86_64
> 
> How reproducible:
> 
> 100%
> 
> Steps to Reproduce:
> 1. echo $TOP

--- Additional comment from Richard W.M. Jones on 2017-02-21 09:26:22 CET ---

(In reply to Anoop C S from comment #2)
> Can you please direct towards this initialization script?

As stated in the original description:

  /etc/bash_completion.d/gluster

from the package glusterfs-cli.  In the source, this file is
called extras/command-completion/gluster.bash

> And how severe is the issue?

Annoying more than severe.  However it prevents building all
the RISC-V upstream software.

--- Additional comment from Niels de Vos on 2017-02-21 09:37:39 CET ---

It looks like "TOP" is not the only variable that leaks:

From <glusterfs.git>/extras/command-completion/gluster.bash:
285 declare FINAL_LIST=''
286 declare LIST=''
287 declare -i TOP=0

Comment 1 Shyamsundar 2018-06-20 18:28:57 UTC
This bug reported is against a version of Gluster that is no longer maintained
(or has been EOL'd). See https://www.gluster.org/release-schedule/ for the
versions currently maintained.

As a result this bug is being closed.

If the bug persists on a maintained version of gluster or against the mainline
gluster repository, request that it be reopened and the Version field be marked
appropriately.

Comment 2 Richard W.M. Jones 2018-06-20 18:51:31 UTC
Wouldn't be too much effort for you to actually fix things instead
of posting such a stupid message?

As a quick search would show you the problem is still
present in the latest version:

https://github.com/gluster/glusterfs/blob/0751039eadb23b8658423477aa81964b339b4f5e/extras/command-completion/gluster.bash#L287

and also present in glusterfs-cli-3.13.2-3.fc28.x86_64

Comment 3 Mark Mielke 2018-08-02 09:22:26 UTC
I'm confused by the several bug related to this as to which one is tracking the need for a fix. In any case, adding my comment here as well...

We're hitting this as well:

-bash-4.2$ cat /etc/system-release
Red Hat Enterprise Linux Server release 7.2 (Maipo)
-bash-4.2$ rpm -q glusterfs-cli
glusterfs-cli-3.7.1-16.el7.x86_64
-bash-4.2$ TOP=/abc
-bash: /abc: syntax error: operand expected (error token is "/abc")
-bash-4.2$ grep "declare -i" /etc/bash_completion.d/gluster 
declare -i TOP=0
-bash-4.2$ declare +i TOP
-bash-4.2$ TOP=/abc
-bash-4.2$ 

It's not just that it is leaked, but that it is declared with "+i" causing bash to interpret it in an integer context from the on.

This needs to be fixed. We have build scripts that set TOP=/path and users upgrading from RHEL 6 to RHEL 7 are hitting this problem now.

Remove glusterfs-cli doesn't seem to be a good option either as even though we don't really use gluster, we do use libvirt, and libvirt ends up pulling in glusterfs-cli.

Comment 4 Anoop C S 2018-08-14 04:31:32 UTC
@Mark,

Sorry for the confusion.

This bug report was initially cloned for v3.10 and subsequently got closed as it reached EOL. After re-opening the version was changed to "mainline" even though we had bug #1425325 opened for tracking in master branch.

I am changing the version to v4.1 for tracking the backport once it is merged in master via bug #1425325.

Comment 5 Worker Ant 2018-08-17 05:32:56 UTC
REVISION POSTED: https://review.gluster.org/20752 (Bash integration script should namespace variables) posted (#2) for review on release-4.1 by Anoop C S

Comment 6 Worker Ant 2018-08-17 05:33:08 UTC
REVIEW: https://review.gluster.org/20752 (Bash integration script should namespace variables) posted (#2) for review on release-4.1 by Anoop C S

Comment 7 Worker Ant 2018-08-21 14:14:07 UTC
COMMIT: https://review.gluster.org/20752 committed in release-4.1 by "Shyamsundar Ranganathan" <srangana@redhat.com> with a commit message- Bash integration script should namespace variables

In the original submitted script, it looks like there was effort
put into namespacing all global variables. However a few mistakes
remained.

GLUSTER_TOP_SUBOPTIONSx were defined, but TOP_SUBOPTIONSx were
referenced. This was likely an unrecognized defect in the
original code submission? These are now corrected to refer to
GLUSTER_TOP_SUBOPTIONSx.

FINAL_LIST, LIST, and TOP were leaked into all Bash shells and
used by the command completion functions. The most problematic
of these was TOP, which was declared with "-i" making it an
integer. This cause other code which used TOP to define a path
to fail like this:

    $ bash
    $ TOP=/abc
    bash: /abc: syntax error: operand expected (error token is "/abc")

These are now qualified as GLUSTER_FINAL_LIST, GLUSTER_LIST, and
GLUSTER_TOP to reduce impact on scripts that might choose to use
these extremely common variable names.

> Change-Id: Ic96eda8efd1f3238bbade6c6ddb69118e8d82158
> Signed-off-by: Mark Mielke <mark.mielke@gmail.com>

(cherry picked from commit 89545e745e4075845c18078be67a31dea93a4e88)

Change-Id: Ic96eda8efd1f3238bbade6c6ddb69118e8d82158
Fixes: bz#1425326
Signed-off-by: Mark Mielke <mark.mielke@gmail.com>

Comment 8 Shyamsundar 2018-08-29 12:44:28 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-4.1.3, please open a new bug report.

glusterfs-4.1.3 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] https://lists.gluster.org/pipermail/announce/2018-August/000111.html
[2] https://www.gluster.org/pipermail/gluster-users/


Note You need to log in before you can comment on or make changes to this bug.