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

The perf.groovy script is not escaping the PR title properly when it contains double-quotes #10654

Closed
tannergooding opened this issue Jul 10, 2018 · 7 comments

Comments

@tannergooding
Copy link
Member

This issue was causing failures in dotnet/coreclr#18849

For example: https://ci2.dot.net/job/dotnet_coreclr/job/perf/job/master/job/perf_perflab_Windows_NT_x64_min_opt_ryujit_smoketest_prtest/2556/

@michellemcdaniel
Copy link
Contributor

@jorive

@jorive
Copy link
Member

jorive commented Jul 10, 2018

@tannergooding This seems a parsing/escaping problem of the batch command line interface, not the submission-metadata.py

@tannergooding
Copy link
Member Author

tannergooding commented Jul 10, 2018

@jorive, potentially.

Looks like the jobs are doing:

if "%GIT_BRANCH:~0,7%" == "origin/" (set "GIT_BRANCH_WITHOUT_ORIGIN=%GIT_BRANCH:origin/=%") else (set "GIT_BRANCH_WITHOUT_ORIGIN=%GIT_BRANCH%")
set "BENCHVIEWNAME=coreclr private %BenchviewCommitName%"
set "BENCHVIEWNAME=%BENCHVIEWNAME:"=""%"
py "%WORKSPACE%\Microsoft.BenchView.JSONFormat\tools\submission-metadata.py" --name "%BENCHVIEWNAME%" --user-email "dotnet-bot@microsoft.com"
py "%WORKSPACE%\Microsoft.BenchView.JSONFormat\tools\build.py" git --branch %GIT_BRANCH_WITHOUT_ORIGIN% --type private

The second set "BENCHVIEWNAME=..." is attempting to replace all " with "". But something else has already changed " to \", which breaks the overall parsing

@jorive
Copy link
Member

jorive commented Jul 10, 2018

the perf.groovy fille is trying to do the operations below, but something is not working as expected

@if not defined _echo echo off
setlocal
  rem Case 1: Double quotes
  if exist ".\submission-metadata.json" del /f /q ".\submission-metadata.json"

  set "GV_TITLE=coreclr private Fixing encodeXmmRegAsIval to ensure the result meets the "fits in imm8" check"
  set "GV_TITLE=%GV_TITLE:"=""%"

  py.exe submission-metadata.py --name "%GV_TITLE%" --user-email "dotnet-bot@microsoft.com" || exit /b 1
  echo/ Case 1
  type submission-metadata.json

  rem Case 2: Angle brackets
  if exist ".\submission-metadata.json" del /f /q ".\submission-metadata.json"

  set "GV_TITLE=coreclr private Fixing encodeXmmRegAsIval to ensure the result meets the <fits in imm8> check"
  rem set "GV_TITLE=%GV_TITLE:"=""%"
  rem set "GV_TITLE=%GV_TITLE:<=^<%"
  rem set "GV_TITLE=%GV_TITLE:>=^>%"

  py.exe submission-metadata.py --name "%GV_TITLE%" --user-email "dotnet-bot@microsoft.com" || exit /b 1
  echo/ Case 2
  type submission-metadata.json
endlocal& exit /b 0

@jorive jorive changed the title The submission-metadata.py script breaks if your PR title contains double-quotes The perf.groovy script is not escaping the PR title properly when it contains double-quotes Jul 10, 2018
@jorive
Copy link
Member

jorive commented Jul 10, 2018

This generated command seems wrong:
py "D:\j\w\perf_perflab_---988491d2\Microsoft.BenchView.JSONFormat\tools\submission-metadata.py" --name "coreclr private Fixing encodeXmmRegAsIval to ensure the result meets the \""fits in imm8\"" check" --user-email "dotnet-bot@microsoft.com"

In addition, the run should have stopped/failed at this point.

@jorive
Copy link
Member

jorive commented Jul 10, 2018

@tannergooding You are right, the generated command in ci2 looks fine, but the executed command displayed in the log is different.

rem In ci2
set "BENCHVIEWNAME=coreclr private %BenchviewCommitName%"
set "BENCHVIEWNAME=%BENCHVIEWNAME:"=""%"
py "%WORKSPACE%\Microsoft.BenchView.JSONFormat\tools\submission-metadata.py" --name "%BENCHVIEWNAME%" --user-email "dotnet-bot@microsoft.com"
rem In Log
set "BENCHVIEWNAME=coreclr private Fixing encodeXmmRegAsIval to ensure the result meets the \"fits in imm8\" check"
set "BENCHVIEWNAME=coreclr private Fixing encodeXmmRegAsIval to ensure the result meets the \""fits in imm8\"" check"
py "D:\j\w\perf_perflab_---988491d2\Microsoft.BenchView.JSONFormat\tools\submission-metadata.py" --name "coreclr private Fixing encodeXmmRegAsIval to ensure the result meets the \""fits in imm8\"" check" --user-email "dotnet-bot@microsoft.com"

Maybe the environment variable BenchviewCommitName should be escaped first.

@jkotas
Copy link
Member

jkotas commented May 23, 2019

perf.groovy is gone

@jkotas jkotas closed this as completed May 23, 2019
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants