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

JENKINS-28188 StringIndexOutOfBoundsException when no root cause #49

Closed
wants to merge 2 commits into from

Conversation

dcoraboeuf
Copy link

When a build has no root cause (possible when firing the job programmatically), the code at org.jenkinsci.plugins.envinject.service.BuildCauseRetriever#buildCauseEnvironmentVariables produces a StringIndexOutOfBoundsException because the substring method is applied to an empty string.

The fix I propose is very basic and could be simplified using Guava - but I didn't date to use it in this plugin.

@oleg-nenashev
Copy link
Member

I suppose StringUtils.join() could resolve the issue in a more elegant way. BTW, we would still need a cycle (and null/empty string checks), so LGTM

@dcoraboeuf
Copy link
Author

Yes, StringUtils, of course :( I'll adjust accordingly.

@dcoraboeuf
Copy link
Author

Done.

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@dcoraboeuf
Copy link
Author

Tests are failing in Cloudbees, not locally. I'll have a look later on.

@dcoraboeuf
Copy link
Author

The failure has nothing to do with this PR - it was already there before.

@oleg-nenashev
Copy link
Member

LGTM. Thanks for the fix

@lanwen
Copy link
Member

lanwen commented May 3, 2015

This is same as #45 (there is test for it)

@oleg-nenashev
Copy link
Member

Closing in a favor of #45
@dcoraboeuf thanks for you efforts. The original title in #45 was too narrow specific, so unfortunately I have not noticed a duplication on early stages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants