Add native support for Eclipse-BundleShape: dir in Equinox framework#1168
Add native support for Eclipse-BundleShape: dir in Equinox framework#1168
Conversation
b6c7f4b to
1be2c2c
Compare
Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
1be2c2c to
5590ead
Compare
Test Results 810 files ±0 810 suites ±0 1h 28m 37s ⏱️ - 1m 24s For more details on these failures, see this check. Results for commit 5590ead. ± Comparison against base commit e3c0da8. |
|
@tjwatson I'm struggling a bit with getting the test working but would you be fine with the general idea? |
- Move bundle shape check to Generation.installBundleFile() method - Read headers only once by creating BundleFile first, then checking headers - Extract JAR to directory only when needed based on Eclipse-BundleShape: dir header - Addresses performance concerns raised in code review Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
tjwatson
left a comment
There was a problem hiding this comment.
I would prefer we didn't have to write new JAR extraction code when something already exists on BundleFile to extract the content. I approve the PR, but would like us to consider removing the new extract code.
|
|
||
| try { | ||
| // Extract to temporary directory first | ||
| getStorage().extractJarToDirectory(jarFile, tempExtractDir); |
There was a problem hiding this comment.
Could use initialBundleFile.getFile("", false) to get the extracted content instead of having a new extractJarToDirectory. I'll admit the implementation of getFile when using the empty string root isn't the most efficient implementation and that could be improved.
tjwatson
left a comment
There was a problem hiding this comment.
Except we seem to have a compilation error.
Problem
The
Eclipse-BundleShape: dirmanifest header is currently only evaluated by P2 during installation time. This creates significant challenges when using Equinox in embedded scenarios:BundleContext#installBundle(String, InputStream), it's impossible to work around this limitationSolution
This PR adds native support for
Eclipse-BundleShape: dirdirectly in the Equinox framework. When a bundle is installed (via eitherinstallBundle(String)orinstallBundle(String, InputStream)), the framework now:Eclipse-BundleShapeheader"dir", extracts the JAR contents to a directory instead of copying the JAR fileDirBundleFileautomaticallyImplementation Details
Modified:
BundleInfo.javaGeneration.installBundleFile()method that combines content setting with bundle shape checkingGeneration.checkBundleShape()method that creates the BundleFile, reads headers, and extracts if neededModified:
Storage.javaECLIPSE_BUNDLESHAPEconstant for the header nameextractJarToDirectory()helper method to safely extract JAR contents with directory traversal protectioninstall()andupdate()methods to useinstallBundleFile()instead of separatesetContent()andgetBundleFile().open()callsThe implementation integrates seamlessly into the existing bundle installation flow, requiring no changes to public APIs or calling code. The bundle's JAR file is opened only once during installation, with headers read from the already-open BundleFile to determine if extraction is needed.
Performance
The refactored implementation ensures optimal performance:
Testing
Added comprehensive test coverage:
test.bundleshape.dirwithEclipse-BundleShape: dirheadertestBundleShapeDir()to verify installation via location stringtestBundleShapeDirWithInputStream()to verify installation via InputStreamBackward Compatibility
This change is fully backward compatible:
Type.REFERENCE) are not affectedType.CONNECT) are not affectedSecurity
The extraction process includes canonical path validation to prevent directory traversal attacks from malicious bundle entries.
Fixes #1167
Original prompt
Fixes #1167
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.