Lucene - Core
  1. Lucene - Core
  2. LUCENE-4206

Allow check-forbidden-apis to also investigate calls to subclasses of forbidden APIs

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-BETA, Trunk
    • Component/s: general/build
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Followup for LUCENE-4202: I think I found a solution, that only adds overhead of parsing all classes in FileSet 2 times (instead of one time) and dynamic investigation of system classes from classloader on demand.

      1. LUCENE-4206.patch
        20 kB
        Uwe Schindler
      2. LUCENE-4206.patch
        21 kB
        Uwe Schindler

        Issue Links

          Activity

          Hide
          Uwe Schindler added a comment -

          Rewrite of the algorithm to also check superclasses and interfaces. The overhead approx doubles the time needed before, but is more thorough.

          Show
          Uwe Schindler added a comment - Rewrite of the algorithm to also check superclasses and interfaces. The overhead approx doubles the time needed before, but is more thorough.
          Hide
          Dawid Weiss added a comment -
          +        public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
          +          final Method m = new Method(name, desc);
          

          Umm... you just stack all methods without considering their access flags?

          Show
          Dawid Weiss added a comment - + public MethodVisitor visitMethod( int access, String name, String desc, String signature, String [] exceptions) { + final Method m = new Method(name, desc); Umm... you just stack all methods without considering their access flags?
          Hide
          Uwe Schindler added a comment -

          We must do this on top-level, otherwise it's different to what I did before? When it then dives to superclasses/interfaces, it assumes that the original compiler already did the check.

          Show
          Uwe Schindler added a comment - We must do this on top-level, otherwise it's different to what I did before? When it then dives to superclasses/interfaces, it assumes that the original compiler already did the check.
          Hide
          Uwe Schindler added a comment -

          And your copypaste of the code is not referring to the actual recursion code, this one only collects all methods/fields - and that must visit all methods, otherwise it would not work - it is a unchanged replacement for the ineffective code of the original "ClassNode extends ClassVisitor" of ASM 4.0. Maybe you shoud apply the patch first

          Show
          Uwe Schindler added a comment - And your copypaste of the code is not referring to the actual recursion code, this one only collects all methods/fields - and that must visit all methods, otherwise it would not work - it is a unchanged replacement for the ineffective code of the original "ClassNode extends ClassVisitor" of ASM 4.0. Maybe you shoud apply the patch first
          Hide
          Uwe Schindler added a comment -

          Small de-spaghettization of the readClass method (inlined).

          I think it's ready to commit for robert, so he cann add his other checks.

          Show
          Uwe Schindler added a comment - Small de-spaghettization of the readClass method (inlined). I think it's ready to commit for robert, so he cann add his other checks.
          Hide
          Robert Muir added a comment -

          I have no idea how to review (its all black magic to me). I'm just glad Uwe didn't give up!

          Show
          Robert Muir added a comment - I have no idea how to review (its all black magic to me). I'm just glad Uwe didn't give up!
          Hide
          Uwe Schindler added a comment -

          Committed trunk revision: 1359827
          Committed 4.x revision: 1359828

          Show
          Uwe Schindler added a comment - Committed trunk revision: 1359827 Committed 4.x revision: 1359828
          Hide
          Hoss Man added a comment -

          hoss20120711-manual-post-40alpha-change

          Show
          Hoss Man added a comment - hoss20120711-manual-post-40alpha-change
          Hide
          Uwe Schindler added a comment -

          I just wanted to add my recent blog post on this issue as further documentation: http://blog.thetaphi.de/2012/07/default-locales-default-charsets-and.html

          Show
          Uwe Schindler added a comment - I just wanted to add my recent blog post on this issue as further documentation: http://blog.thetaphi.de/2012/07/default-locales-default-charsets-and.html

            People

            • Assignee:
              Uwe Schindler
              Reporter:
              Uwe Schindler
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development