Bug 1085761 - fedora-review incorrect env unset in review-env.sh script
Summary: fedora-review incorrect env unset in review-env.sh script
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Fedora
Classification: Fedora
Component: fedora-review
Version: 20
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Stanislav Ochotnicky
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-04-09 09:48 UTC by Pavel Alexeev
Modified: 2015-04-15 14:06 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-04-15 14:03:32 UTC


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Bugzilla 1185565 None None None Never

Internal Links: 1185565

Description Pavel Alexeev 2014-04-09 09:48:52 UTC
Description of problem:
$ LANG=C fedora-review -b 1084397
…
WARNING: Illegal return from /usr/share/fedora-review/scripts/generic-large-data.sh, code 80, output: stdout:None stderr:./review-env.sh: line 7: unset: `:DO': not a valid identifier
./review-env.sh: line 7: unset: `:cd': not a valid identifier
./review-env.sh: line 7: unset: `:do': not a valid identifier
./review-env.sh: line 7: unset: `:le': not a valid identifier
./review-env.sh: line 7: unset: `:li#79:co#253:am:xn:xv:LP:sr': not a valid identifier
./review-env.sh: line 7: unset: `:cs': not a valid identifier
./review-env.sh: line 7: unset: `:im': not a valid identifier
./review-env.sh: line 7: unset: `:ke': not a valid identifier
./review-env.sh: line 7: unset: `:ti': not a valid identifier
./review-env.sh: line 7: unset: `:se': not a valid identifier
./review-env.sh: line 7: unset: `:Co#256:pa#64:AF': not a valid identifier
./review-env.sh: line 7: unset: `:vb': not a valid identifier
./review-env.sh: line 7: unset: `:ac': not a valid identifier
./review-env.sh: line 7: unset: `:po': not a valid identifier
./review-env.sh: line 7: unset: `:k3': not a valid identifier
./review-env.sh: line 7: unset: `:k8': not a valid identifier
./review-env.sh: line 7: unset: `:F3': not a valid identifier
./review-env.sh: line 7: unset: `:F7': not a valid identifier
./review-env.sh: line 7: unset: `:K2': not a valid identifier
./review-env.sh: line 7: unset: `:*7': not a valid identifier
./review-env.sh: line 7: unset: `:%e': not a valid identifier
./review-env.sh: line 7: unset: `:@7': not a valid identifier
./review-env.sh: line 7: unset: `:kd': not a valid identifier

It come from line 7 aforementioned script:
unset $(env | sed -n 's/=.*//p')

It does not account what more then one line variables body may be set:
$ env | fgrep -A23 TERMCAP=
TERMCAP=SC|screen|VT 100/ANSI X3.64 virtual terminal:\
        :DO=\E[%dB:LE=\E[%dD:RI=\E[%dC:UP=\E[%dA:bs:bt=\E[Z:\
        :cd=\E[J:ce=\E[K:cl=\E[H\E[J:cm=\E[%i%d;%dH:ct=\E[3g:\
        :do=^J:nd=\E[C:pt:rc=\E8:rs=\Ec:sc=\E7:st=\EH:up=\EM:\
        :le=^H:bl=^G:cr=^M:it#8:ho=\E[H:nw=\EE:ta=^I:is=\E)0:\
        :li#79:co#253:am:xn:xv:LP:sr=\EM:al=\E[L:AL=\E[%dL:\
        :cs=\E[%i%d;%dr:dl=\E[M:DL=\E[%dM:dc=\E[P:DC=\E[%dP:\
        :im=\E[4h:ei=\E[4l:mi:IC=\E[%d@:ks=\E[?1h\E=:\
        :ke=\E[?1l\E>:vi=\E[?25l:ve=\E[34h\E[?25h:vs=\E[34l:\
        :ti=\E[?1049h:te=\E[?1049l:us=\E[4m:ue=\E[24m:so=\E[3m:\
        :se=\E[23m:mb=\E[5m:md=\E[1m:mr=\E[7m:me=\E[m:ms:\
        :Co#256:pa#64:AF=\E[3%dm:AB=\E[4%dm:op=\E[39;49m:AX:\
        :vb=\Eg:G0:as=\E(0:ae=\E(B:\
        :ac=\140\140aaffggjjkkllmmnnooppqqrrssttuuvvwwxxyyzz{{||}}~~..--++,,hhII00:\
        :po=\E[5i:pf=\E[4i:Km=\E[M:k0=\E[10~:k1=\EOP:k2=\EOQ:\
        :k3=\EOR:k4=\EOS:k5=\E[15~:k6=\E[17~:k7=\E[18~:\
        :k8=\E[19~:k9=\E[20~:k;=\E[21~:F1=\E[23~:F2=\E[24~:\
        :F3=\E[1;2P:F4=\E[1;2Q:F5=\E[1;2R:F6=\E[1;2S:\
        :F7=\E[15;2~:F8=\E[17;2~:F9=\E[18;2~:FA=\E[19;2~:kb=:\
        :K2=\EOE:kB=\E[Z:kF=\E[1;2B:kR=\E[1;2A:*4=\E[3;2~:\
        :*7=\E[1;2F:#2=\E[1;2H:#3=\E[2;2~:#4=\E[1;2D:%c=\E[6;2~:\
        :%e=\E[5;2~:%i=\E[1;2C:kh=\E[1~:@1=\E[1~:kH=\E[4~:\
        :@7=\E[4~:kN=\E[6~:kP=\E[5~:kI=\E[2~:kD=\E[3~:ku=\EOA:\
        :kd=\EOB:kr=\EOC:kl=\EOD:km:

Variant which should solve it:
unset $( env | sed -n 's/=.*//p' | egrep -v '^\s' )

Version-Release number of selected component (if applicable):
$ rpm -q fedora-review
fedora-review-0.5.1-2.fc20.noarch

How reproducible:
Always

Comment 1 Alec Leamas 2014-04-09 10:25:20 UTC
Nice catch!

That said, solution seems to either trust continuation lines to be indented as in your proposal, or trust environment variables name to be sane by filtering with something like grep -E '^[a-zA-Z0-9_]+='. None of these solutions are without loopholes, though(?)

Perhaps making an assumption about "sane" environment variables is a little more safe. For one thing, whatever other things there are in the env those will then not be unset nut rather silently ignored. Perhaps better, dunno.

Comment 2 Florian Weimer 2014-10-06 13:15:19 UTC
This also breaks more often with bash-4.2.51-2.fc20.x86_64 due the function name mangling patch.

Comment 3 Alec Leamas 2014-10-14 11:56:39 UTC
Fixed upstream in https://fedorahosted.org/FedoraReview/changeset/18d98aa3d0 Thanks for the 'env -i' hint!

Comment 4 Orion Poplawski 2014-10-30 15:49:33 UTC
Can we get a new release for F20 (and probably F19) then?

Comment 5 Orion Poplawski 2015-01-31 04:36:01 UTC
Also seeing:
WARNING: Illegal return from /usr/share/fedora-review/scripts/java-check-bundled-jars.sh, code 81, output: stdout:Jar files in source (see attachment)
 stderr:./review-env.sh: line 7: unset: `BASH_FUNC_module()': not a valid identifier

fedora-review-0.5.2-2.fc21.noarch

Comment 6 Gerard Ryan 2015-03-01 17:35:55 UTC
(In reply to Orion Poplawski from comment #5)
> [...]
> fedora-review-0.5.2-2.fc21.noarch

I'm also seeing this in this version in F21:

WARNING: Illegal return from /usr/share/fedora-review/scripts/generic-excludearch.sh, code 82, output: stdout:None stderr:./review-env.sh: line 7: unset: `BASH_FUNC_module()': not a valid identifier
./review-env.sh: line 7: unset: `BASH_FUNC_scl()': not a valid identifier
./review-env.sh: line 7: unset: `[': not a valid identifier
./review-env.sh: line 7: unset: `"$CMD"': not a valid identifier

WARNING: Illegal return from /usr/share/fedora-review/scripts/generic-large-docs.sh, code 82, output: stdout:Documentation size is 10240 bytes in 1 files.
 stderr:./review-env.sh: line 7: unset: `BASH_FUNC_module()': not a valid identifier
./review-env.sh: line 7: unset: `BASH_FUNC_scl()': not a valid identifier
./review-env.sh: line 7: unset: `[': not a valid identifier
./review-env.sh: line 7: unset: `"$CMD"': not a valid identifier

WARNING: Illegal return from /usr/share/fedora-review/scripts/generic-srv-opt.sh, code 80, output: stdout:None stderr:./review-env.sh: line 7: unset: `BASH_FUNC_module()': not a valid identifier
./review-env.sh: line 7: unset: `BASH_FUNC_scl()': not a valid identifier
./review-env.sh: line 7: unset: `[': not a valid identifier
./review-env.sh: line 7: unset: `"$CMD"': not a valid identifier

WARNING: Illegal return from /usr/share/fedora-review/scripts/generic-large-data.sh, code 83, output: stdout:None stderr:./review-env.sh: line 7: unset: `BASH_FUNC_module()': not a valid identifier
./review-env.sh: line 7: unset: `BASH_FUNC_scl()': not a valid identifier
./review-env.sh: line 7: unset: `[': not a valid identifier
./review-env.sh: line 7: unset: `"$CMD"': not a valid identifier

Comment 8 Alec Leamas 2015-04-15 14:06:41 UTC
Oops! Not the commits in #comment7, but this one:

https://fedorahosted.org/FedoraReview/changeset/18d98aa3d0f441f7cd4732


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