karaf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jamie G." <jamie.goody...@gmail.com>
Subject Re: git commit: [KARAF-2753] Logging for override mechanism. Added additional logging and unit test to trigger log events
Date Wed, 12 Feb 2014 14:13:10 GMT
True, I'll make the adjustment.

--jamie


On Wed, Feb 12, 2014 at 10:40 AM, Achim Nierbeck <bcanhome@googlemail.com>wrote:

> 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 <jgoodyear@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 <jgoodyear@apache.org>
> > Authored: Wed Feb 12 10:29:10 2014 -0330
> > Committer: jgoodyear <jgoodyear@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/>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message