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

Convert GString to String when bridging to jelly. #202

Merged
merged 1 commit into from
Dec 22, 2020

Conversation

Vlatombe
Copy link
Member

@Vlatombe Vlatombe commented Dec 22, 2020

When using a groovy view and passing a groovy expression as
"${e1}${e2}" to a taglib, the resulting value is not coerced to a
String, which can lead to incorrect results when calling functions from
within jelly.

For example, when declaring a widget in Jenkins, using the following index.groovy as view

def l = namespace(lib.LayoutTagLib);

l.pane(width:2, title:"Title", id:"${my.urlName}", collapsedText:"CollapsedText") {
  text('Foo')
}

In the resulting map that is passed to JellyBuilder, id has type GStringImpl, unlike title or collapsedText which get coerced to String.

Then in https://github.com/jenkinsci/jenkins/blob/62546443947fafecc94a5e3b47bb9129bed8e148/core/src/main/resources/lib/layout/pane.jelly#L51, the function call doesn't happen because attr.id is a GString, not a String.

@jglick

When using a groovy view and passing a groovy expression as
"${e1}${e2}" to a taglib, the resulting value is not coerced to a
String, which can lead to incorrect results when calling functions from
within jelly.
@Vlatombe Vlatombe changed the title Convert GString to string when bridging to jelly. Convert GString to String when bridging to jelly. Dec 22, 2020
@jglick jglick added the bug label Dec 22, 2020
@jglick jglick merged commit e096bd4 into jenkinsci:master Dec 22, 2020
@Vlatombe Vlatombe deleted the gstring-to-string branch December 22, 2020 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants