[type:fix] validate plugin jar Maven metadata - #7198
dengliming wants to merge 3 commits into
Conversation
Aias00
left a comment
There was a problem hiding this comment.
Good defensive fix with a clear failure mode.
Previously properties.get("version").toString() threw a raw NullPointerException whenever pom.properties lacked one of the three keys — an opaque error deep inside jar parsing. Using getProperty(...) returns null instead, and the new post-parse check converts that into an explicit ShenyuException("plugin jar is missing required Maven metadata: groupId, artifactId and version"), which is far easier to act on.
Notes:
- The validation is placed after the jar loop, so it also catches the case where no
pom.propertiesexists at all (all three fields stay null) — that's the most common real-world failure and it's now handled. StringUtils.isAnyBlankcovers null, empty and whitespace, so a properties file withversion=is rejected rather than producing an emptyjarKey.- The new
PluginJarParserTestis genuinely useful: it builds real in-memory jars (with full metadata, with none, and with partial metadata) and asserts the throw for the latter two. Much better coverage than this class had.
One non-blocking consideration: parseJar now throws for jars without Maven metadata, whereas before it would NPE — so any existing ext-plugin jar built without pom.properties moves from "broken at load time" to "rejected with a clear message". That's the right trade-off, but it is a behaviour change worth a line in the release notes if ext-plugin jars are commonly hand-built.
Fixes #6631.\n\nRead Maven coordinates without dereferencing missing properties and reject jars that lack a complete groupId/artifactId/version tuple before a null cache key can be created. Adds coverage for valid, absent, and incomplete metadata.\n\nTests:
./mvnw -q -pl shenyu-web -am -DskipTests=false -Dcheckstyle.skip=false -Dtest=PluginJarParserTest -DfailIfNoTests=false test