Skip to content

[type:fix] validate plugin jar Maven metadata - #7198

Open
dengliming wants to merge 3 commits into
apache:masterfrom
dengliming:fix-6631-validate-plugin-metadata
Open

dengliming wants to merge 3 commits into
apache:masterfrom
dengliming:fix-6631-validate-plugin-metadata

Conversation

@dengliming

Copy link
Copy Markdown
Member

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

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.properties exists at all (all three fields stay null) — that's the most common real-world failure and it's now handled.
  • StringUtils.isAnyBlank covers null, empty and whitespace, so a properties file with version= is rejected rather than producing an empty jarKey.
  • The new PluginJarParserTest is 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.

This branch has not been deployed

No deployments
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.

[BUG] PluginJarParser.parseJar NPE on missing/incomplete pom.properties

2 participants