| Message ID | 20241210-pks-ci-section-fixes-v2-3-e087cfd174f4@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Random improvements to GitLab CI | expand |
Patrick Steinhardt <ps@pks.im> writes: > We use printf to set up sections with GitLab CI, which requires us to > print a bunch of escape sequences via printf. The group name is > controlled by the user and is expanded directly into the formatting > string, which may cause problems in case the argument controls escape > sequences or formatting directives. Could it be you mean "contains" instead of "controls"? > > Fix this potential issue by using formatting directives to pass variable > data. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > ci/lib.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ci/lib.sh b/ci/lib.sh > index a54601be923bf475ba1a9cafd98bb1cb71a10255..f15f77f03a06120afbee438cee76ddc2683e1fa2 100755 > --- a/ci/lib.sh > +++ b/ci/lib.sh > @@ -18,7 +18,7 @@ elif test true = "$GITLAB_CI" > then > begin_group () { > need_to_end_group=t > - printf "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)[collapsed=true]\r\e[0K$1\n" > + printf '\e[0Ksection_start:%s:%s[collapsed=true]\r\e[0K%s\n' "$(date +%s)" "$(echo "$1" | tr ' ' _)" "$1" Personally I find this line rather lengthy and hard to read with all the single and double quotes. So I would suggest to split the line with a backslash and put the arguments on a separate line. But I don't think there's a general guideline on this, so feel free to ignore. > trap "end_group '$1'" EXIT > set -x > } > @@ -27,7 +27,7 @@ then > test -n "$need_to_end_group" || return 0 > set +x > need_to_end_group= > - printf "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K\n" > + printf '\e[0Ksection_end:%s:%s\r\e[0K\n' "$(date +%s)" "$(echo "$1" | tr ' ' _)" Same here. But that's all I've got on this patch series. Looking good! -- Toon
On Wed, Dec 11, 2024 at 01:37:53PM +0100, Toon Claes wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > We use printf to set up sections with GitLab CI, which requires us to > > print a bunch of escape sequences via printf. The group name is > > controlled by the user and is expanded directly into the formatting > > string, which may cause problems in case the argument controls escape > > sequences or formatting directives. > > Could it be you mean "contains" instead of "controls"? Oh, yeah. > > Fix this potential issue by using formatting directives to pass variable > > data. > > > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > > --- > > ci/lib.sh | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/ci/lib.sh b/ci/lib.sh > > index a54601be923bf475ba1a9cafd98bb1cb71a10255..f15f77f03a06120afbee438cee76ddc2683e1fa2 100755 > > --- a/ci/lib.sh > > +++ b/ci/lib.sh > > @@ -18,7 +18,7 @@ elif test true = "$GITLAB_CI" > > then > > begin_group () { > > need_to_end_group=t > > - printf "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)[collapsed=true]\r\e[0K$1\n" > > + printf '\e[0Ksection_start:%s:%s[collapsed=true]\r\e[0K%s\n' "$(date +%s)" "$(echo "$1" | tr ' ' _)" "$1" > > Personally I find this line rather lengthy and hard to read with all the > single and double quotes. So I would suggest to split the line with a > backslash and put the arguments on a separate line. But I don't think > there's a general guideline on this, so feel free to ignore. Fair, will do. > > trap "end_group '$1'" EXIT > > set -x > > } > > @@ -27,7 +27,7 @@ then > > test -n "$need_to_end_group" || return 0 > > set +x > > need_to_end_group= > > - printf "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K\n" > > + printf '\e[0Ksection_end:%s:%s\r\e[0K\n' "$(date +%s)" "$(echo "$1" | tr ' ' _)" > > Same here. > > But that's all I've got on this patch series. Looking good! Thanks! Patrick
diff --git a/ci/lib.sh b/ci/lib.sh index a54601be923bf475ba1a9cafd98bb1cb71a10255..f15f77f03a06120afbee438cee76ddc2683e1fa2 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -18,7 +18,7 @@ elif test true = "$GITLAB_CI" then begin_group () { need_to_end_group=t - printf "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)[collapsed=true]\r\e[0K$1\n" + printf '\e[0Ksection_start:%s:%s[collapsed=true]\r\e[0K%s\n' "$(date +%s)" "$(echo "$1" | tr ' ' _)" "$1" trap "end_group '$1'" EXIT set -x } @@ -27,7 +27,7 @@ then test -n "$need_to_end_group" || return 0 set +x need_to_end_group= - printf "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K\n" + printf '\e[0Ksection_end:%s:%s\r\e[0K\n' "$(date +%s)" "$(echo "$1" | tr ' ' _)" trap - EXIT } else
We use printf to set up sections with GitLab CI, which requires us to print a bunch of escape sequences via printf. The group name is controlled by the user and is expanded directly into the formatting string, which may cause problems in case the argument controls escape sequences or formatting directives. Fix this potential issue by using formatting directives to pass variable data. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- ci/lib.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)