Your browser may have trouble rendering this page. See supported browsers for more information.

|<<>>|60 of 273 Show listMobile Mode

A dynamically generated Groovy foot-gun

Published by marco on

Groovy is a dynamically typed programming language that executes on the Java Runtime. It mixes its own highly dynamic syntax with islands of Java code. The Android ecosystem and its IDE use Gradle for its build scripts. Gradle uses the Groovy programming language.

The Problem Code

A large project I’m working on contains quite a bit of custom Gradle code for integrating framework libraries, making obfuscated builds, configuring publication, and, finally, creating signed builds.

The signed builds are configured using standard Android Gradle DSL commands. Basically, there was a block of code something like the one shown below.

signing {
  storeFile = getKeyStoreFile()
  storePassword = getKeyStorePassword()
  keyAlias = getKeyAlias()
  keyPassword = getKeyPassword()
}

The names of the methods (e.g. getKeyAlias) used to be different before I’d refactored them to have more standard names.[1] The methods check whether there are environment variables set by the build server, using sane defaults for developer builds.[2]

This is where I went wrong. Never touch a running system[3], even when you’re trying to pull it back from the precipice of “maintenance nightmare that everyone is terrified to touch, to say nothing of change”. Well, I changed it, and ended up frittering away a couple of hours investigating the Groovy “feature” outlined below.

Lowering Groovy to Java

Groovy performs syntax-checking, but is extremely lenient as far types and variables are concerned. Variables have to be defined, but pretty much anything can be coerced into anything else. It is transformed to Java code and then Java byte code by the Java compiler. Any typing errors you see are from the Java compiler, not the

As any programming language would, Groovy resolves identifiers to match the declaration that is closest in scope to the call, even when that declaration is generated at compile-time and couldn’t possibly be the one that the original author had intended to call. This is going to be important later (which is why I put it in scary italics).

The four methods above are defined in an ext {} block[4]. Calling them without a specific target as above automatically resolves to the methods from the ext {} block.

Oddly, of the four properties being set in the example above, only the first two actually called the methods I’d defined in the ext {} block. The calls to getKeyAlias() and getKeyPassword() were not made to the expected functions. I could tell they weren’t being called because the logger.info() calls from those two methods never appeared in the output.

What the hell is going on? If you look carefully, you’ll notice that the first two methods have different names than you would use for writing the getter and setter for the properties being assigned. The second two methods match those names exactly.

Auto-generated Java Code

When Groovy lowers its syntax to Java code, it declares these getters and setters. The Java compiler, in turn, references these new methods because the calls in the original Groovy code hadn’t been specific about the target of the methods. Instead of lowering to Java code and being explicit about which ext block the method should be called from, Groovy just left the naked call as I’d written it. Probably, if I’d explicitly called ext.getKeyAlias(), it would have avoided calling the dynamically generated this.getKeyAlias() method.

Of course, Groovy had trained me to stop prepended the target ext. on global function calls because ext resolves to different things, depending on the DSL-specific context. Sometimes it’s the root project’s extra variables and sometimes it’s the sub-projects extra variables and sometimes ext doesn’t work at all (e.g. in Java classes, naturally, but also in blocks created by special keywords).

Sure, you can trying playing around with rootProject.ext. or other similar constructs, but the code quickly becomes even more unreadable than it already would be and the non-prefixed version works 99% of the time.

So what ended up happening was that, instead of calling the method I’d actually called, the Groovy compiler generated a new method with the same name and a higher specificity in the scope, capturing the call. Instead of calling my method, it ended up calling this.setKeyAlias(this.getKeyAlias()), which is basically a NOP that leaves the property empty.

Fixing the Problem

The solution is to use a unique name for the function that does not conflict with any of the auto-generated getters. That is, of course, an unmaintainable nightmare, but part and parcel of working with Gradle.

signing {
  storeFile = getKeyStoreFile()
  storePassword = getKeyStorePassword()
  keyAlias = getSigningKeyAlias()
  keyPassword = getSigningKeyPassword()
}

Lo and behold, my log entries appeared and I was back in business.

Fixing the Compiler

The compiler authors could have tried harder to avoid altering the semantics of the higher-level Groovy code when replacing it with Java.

One way would be to use more obfuscated auto-generated getters and setters (to the degree that Java even allows this, which I think it does).

Another way was hinted at above: when lowering calls that auto-resolve to functions declared in ext regions, include information about the resolution in the call made in Java. That is, instead of just encoding getKeyAlias() as I’d written it (which is semantically correct at the Groovy level), transform that call to rootProject.ext.getKeyAlias() in Java.

tl;dr

Gradle is a shaky piece of business that automagically generates code that might replace actual, legitimate calls in your own code. It should never have been used for a build system. It makes MSBuild seem like a pretty good idea.


[1] The code had somewhat haphazard names before, which actually ended up protecting it from the bug I ran into. Lesson learned: using obtuse function names is good.
[2] Yes, we’re going to integrate a secret store like Vault instead of relying on environment variables. That comes after this refactoring.
[3] In the end, I was happy I’d refactored everything because I ended up with a lot less code that was much more self-documenting. The job was to refactor and clean up scripts that had grown over the years, so there was no avoiding renaming oddly named methods. The side-effect in this case was unusually painful.
[4]

Gradle lets you declare “extra” variables in a scoped block called ext.

The parent of this block depends on the context. It’s usually rootProject, unless you’re executing project-specific code, in which case any declarations will be made in the sub-project-specific ext block instead of the one for the rootProject.

It can get quite confusing if you’re not sure which context you’re in when you declare an ext {} block, which is why some authors try to declare rootProject.ext or project.ext, but then you run into problems when you grab a variable from the wrong extra region.

Not to mention that it gets quite a bit messier to read and if all authors don’t stick to the same style, it becomes difficult to tell which explicit references are necessary and which are just thrown in there “to make sure”.

I settled on just declaring as much as possible in the ext {} and letting Groovy figure out which variable to use from scope. That ended up biting me in the ass exactly once, as detailed above.