just one comment from my side, I'd stick to the warn level for all loggings, i think this is something that should show up on an Administrator console just in case. To make sure no "malicious" code is "injected" Cause from my point of view it's quite simple to replace bundle a with c by just "renaming" it to a.minorAdditionAndPatchFixForSomeoneSpecial_I_Love
regards, Achim 2014-02-12 15:00 GMT+01:00 <jgoody...@apache.org>: > Updated Branches: > refs/heads/master d2af093dd -> 36808c560 > > > [KARAF-2753] Logging for override mechanism. Added additional logging and > unit test to trigger log events > > > Project: http://git-wip-us.apache.org/repos/asf/karaf/repo > Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/36808c56 > Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/36808c56 > Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/36808c56 > > Branch: refs/heads/master > Commit: 36808c5607d3fc0de40861146775e10b7c248e59 > Parents: d2af093 > Author: jgoodyear <jgoody...@apache.org> > Authored: Wed Feb 12 10:29:10 2014 -0330 > Committer: jgoodyear <jgoody...@apache.org> > Committed: Wed Feb 12 10:29:10 2014 -0330 > > ---------------------------------------------------------------------- > .../karaf/features/internal/Overrides.java | 25 ++++++++++- > .../karaf/features/internal/OverridesTest.java | 47 ++++++++++++++++++++ > 2 files changed, 71 insertions(+), 1 deletion(-) > ---------------------------------------------------------------------- > > > > http://git-wip-us.apache.org/repos/asf/karaf/blob/36808c56/features/core/src/main/java/org/apache/karaf/features/internal/Overrides.java > ---------------------------------------------------------------------- > diff --git > a/features/core/src/main/java/org/apache/karaf/features/internal/Overrides.java > b/features/core/src/main/java/org/apache/karaf/features/internal/Overrides.java > index 655dfea..8397222 100644 > --- > a/features/core/src/main/java/org/apache/karaf/features/internal/Overrides.java > +++ > b/features/core/src/main/java/org/apache/karaf/features/internal/Overrides.java > @@ -48,6 +48,7 @@ public class Overrides { > private static final Logger LOGGER = > LoggerFactory.getLogger(Overrides.class); > > private static final String OVERRIDE_RANGE = "range"; > + private static final String VENDOR_WARNING = "Malicious code possibly > introduced by patch override, see log for details"; > > /** > * Compute a list of bundles to install, taking into account > overrides. > @@ -86,6 +87,7 @@ public class Overrides { > if (manifest != null) { > String bsn = getBundleSymbolicName(manifest); > Version ver = getBundleVersion(manifest); > + String ven = getBundleVendor(manifest); > String url = info.getLocation(); > for (Clause override : overrides) { > Manifest overMan = > manifests.get(override.getName()); > @@ -111,10 +113,26 @@ public class Overrides { > range = VersionRange.parseVersionRange(vr); > } > > + String vendor = getBundleVendor(overMan); > > + // Before we do a replace, lets check if vendors > change > + if (ven == null) { > + if (vendor != null) { > + LOGGER.warn(VENDOR_WARNING); > + } > + } else { > + if (vendor == null) { > + LOGGER.warn(VENDOR_WARNING); > + } else { > + if (!vendor.equals(ven)) { > + LOGGER.warn(VENDOR_WARNING); > + } > + } > + } > // The resource matches, so replace it with the > overridden resource > // if the override is actually a newer version > than what we currently have > if (range.contains(ver) && ver.compareTo(oVer) < > 0) { > + LOGGER.info("Overriding original bundle " + > url + " to " + override.getName()); > ver = oVer; > url = override.getName(); > } > @@ -178,6 +196,11 @@ public class Overrides { > return bsn; > } > > + private static String getBundleVendor(Manifest manifest) { > + String ven = > manifest.getMainAttributes().getValue(Constants.BUNDLE_VENDOR); > + return ven; > + } > + > private static Manifest getManifest(String url) throws IOException { > InputStream is = new URL(url).openStream(); > try { > @@ -205,4 +228,4 @@ public class Overrides { > } > return cs[0].getName(); > } > -} > \ No newline at end of file > +} > > > http://git-wip-us.apache.org/repos/asf/karaf/blob/36808c56/features/core/src/test/java/org/apache/karaf/features/internal/OverridesTest.java > ---------------------------------------------------------------------- > diff --git > a/features/core/src/test/java/org/apache/karaf/features/internal/OverridesTest.java > b/features/core/src/test/java/org/apache/karaf/features/internal/OverridesTest.java > index 46d163a..79e2015 100644 > --- > a/features/core/src/test/java/org/apache/karaf/features/internal/OverridesTest.java > +++ > b/features/core/src/test/java/org/apache/karaf/features/internal/OverridesTest.java > @@ -42,6 +42,9 @@ public class OverridesTest { > private File b101; > private File b102; > private File b110; > + private File c100; > + private File c101; > + private File c110; > > @Before > public void setUp() throws IOException { > @@ -72,6 +75,50 @@ public class OverridesTest { > .set("Bundle-Version", "1.1.0") > .build(), > new FileOutputStream(b110)); > + > + c100 = File.createTempFile("karafc", "-100.jar"); > + copy(TinyBundles.bundle() > + .set("Bundle-SymbolicName", bsn) > + .set("Bundle-Version", "1.0.0") > + .set("Bundle-Vendor", "Apache") > + .build(), > + new FileOutputStream(c100)); > + > + c101 = File.createTempFile("karafc", "-101.jar"); > + copy(TinyBundles.bundle() > + .set("Bundle-SymbolicName", bsn) > + .set("Bundle-Version", "1.0.1") > + .set("Bundle-Vendor", "NotApache") > + .build(), > + new FileOutputStream(c101)); > + > + c110 = File.createTempFile("karafc", "-110.jar"); > + copy(TinyBundles.bundle() > + .set("Bundle-SymbolicName", bsn) > + .set("Bundle-Version", "1.1.0") > + .set("Bundle-Vendor", "NotApache") > + .build(), > + new FileOutputStream(c110)); > + } > + > + @Test > + public void testDifferentVendors() throws IOException { > + File props = File.createTempFile("karaf", "properties"); > + Writer w = new FileWriter(props); > + w.write(c101.toURI().toString()); > + w.write("\n"); > + w.write(c110.toURI().toString()); > + w.write("\n"); > + w.close(); > + > + List<BundleInfo> res = Overrides.override( > + Arrays.<BundleInfo>asList(new > Bundle(c100.toURI().toString())), > + props.toURI().toString()); > + assertNotNull(res); > + assertEquals(1, res.size()); > + BundleInfo out = res.get(0); > + assertNotNull(out); > + assertEquals(c101.toURI().toString(), out.getLocation()); > } > > @Test > > -- Apache Karaf <http://karaf.apache.org/> Committer & PMC OPS4J Pax Web <http://wiki.ops4j.org/display/paxweb/Pax+Web/> Committer & Project Lead OPS4J Pax for Vaadin <http://team.ops4j.org/wiki/display/PAXVAADIN/Home> Commiter & Project Lead blog <http://notizblog.nierbeck.de/>
