jenkinsci / ghprb-plugin

github pull requests builder plugin for Jenkins
https://plugins.jenkins.io/ghprb/
MIT License
502 stars 608 forks source link

Add option to exclude long description #556

Open rrehbein opened 7 years ago

rrehbein commented 7 years ago

Could an option be added to exclude ghprbPullLongDescription from the build. This seems to be the field that we hit escaping issues with. The issue we see is sometimes the escaping results in commands executing from the description. In our internal PR descriptions, we have started adding testing or verification notes / suggestions, which in one case included a tail -f /var/log/messages command, which executed.

Observed on ghprb-plugin 1.39.0

From what I can tell, we personally don't use the long description in our build process. Till execution of descriptions is resolved, perhaps give us the option to not attempt to pass it through?

Similar issues:

410, which was closed as a duplicate of #327

https://issues.jenkins-ci.org/browse/JENKINS-29919

ramsey commented 7 years ago

We're experiencing the same problem on version 1.39.0 of ghprb-plugin.

This is an example from our console output:

/usr/bin/ant: line 332: 5: command not found
/usr/bin/ant: line 332: 2+2+1: command not found

It appears related to improper escaping of the Markdown contained in the pull request's long description. For example, this is what is passed to ant:

"-DghprbPullLongDescription=Blah blah blah blah blah.\r\n\r\nWhy `5`? Well... No reason in particular, but I wanted it to be more than 2. `2+2+1` seemed reasonable."

Note the backticks around "5" and "2+2+1." This is what appears to be confusing Ant.

modess commented 6 years ago

Same issue here with version 1.39.0, if backticks are used in the description ant fails.

akoeplinger commented 6 years ago

We've worked around this problem by using the EnvInject plugin and setting the ghprbPullLongDescription and ghprbPullTitle variables to empty before the build:

image

ntherning commented 5 years ago

The workaround doesn't seem to resolve this issue when using ant. The PR description, including any backticks, is still passed as -DghprbPullLongDescription=... in the ant command line.

andreyzh commented 5 years ago

This would be nice, since we have ANT as build step, and if any quotes are found on the PR description, the build fails.

bjoernhaeuser commented 5 years ago

Looks to me more like a core jenkins problem in this case, no? This plugin is just passing these information on to jenkins core.

Not sure what the plugin could do better, the core should be reponsible for proper escaping.

rrehbein commented 5 years ago

Agreed, the escaping should be a solved issue, but given the number of bugs related to it not being a solved issue, could we please have an option to not attempt one of the causes that help feed the issue?

For our system, I changed the escapeText function of https://github.com/jenkinsci/ghprb-plugin/blob/4e86ed47a96a01eeaa51a479ff604252109635f6/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java to always return an empty string. My request is to have the option to exclude the problem fields from the attempt, without having to custom compile the plugin.

akhabali commented 2 years ago

I have faced the same issue with the double quotes in the PR description. more details in https://github.com/jenkinsci/ghprb-plugin/issues/364#issuecomment-1055508888