Velocity
  1. Velocity
  2. VELOCITY-736

Introspection regression from 1.5 to 1.6.2

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.6.2
    • Fix Version/s: None
    • Component/s: Engine
    • Labels:
      None
    • Environment:
      Sun JDK 1.5

      Description

      When upgrading from Velcocity 1.5 to 1.6.2, the following snippet of code changed behavior.

      In Velocity 1.5, the output was:

      The file upload exceeded 100

      In Velocity 1.6.2, the output is:

      The file upload exceeded $ex.permittedSize

      There is nothing in the velocity log file to help me identify why it's not resolving 'permittedSize' to the correct bean method.

      Here is a test program to replicate the problem. The context variable in question is the Commons FileUpload exception class documented here:

      http://commons.apache.org/fileupload/apidocs/org/apache/commons/fileupload/FileUploadBase.SizeLimitExceededException.html

      I am using commons-fileupload-1.2.jar

      import java.io.StringWriter;
      import org.apache.commons.fileupload.FileUploadBase;
      import org.apache.velocity.VelocityContext;
      import org.apache.velocity.app.VelocityEngine;
      import org.apache.velocity.context.Context;

      public class Main {
      public static void main(String[] args) throws Exception

      { VelocityEngine e = new VelocityEngine(); String testTemplate = "The file upload exceeded $ex.permittedSize"; StringWriter out = new StringWriter(); Context ctx = new VelocityContext(); FileUploadBase.FileSizeLimitExceededException ex = new FileUploadBase.FileSizeLimitExceededException("too big!", 50, 100); ctx.put("ex",ex); e.evaluate(ctx, out, "Tester", testTemplate); System.out.println(out.toString()); }

      }

        Issue Links

          Activity

          Hide
          Christoph Reck added a comment -

          Interesting why this worked in 1.5...

          The api doc lists for Class FileUploadBase.SizeLimitExceededException:
          Methods inherited from class org.apache.commons.fileupload.FileUploadBase.SizeException
          getActualSize, getPermittedSize

          And Class FileUploadBase.SizeException has:
          Method Summary
          long getActualSize()
          long getPermittedSize()

          which means these are package private and therefore not allowed to be used externally.

          Can someone independently try it in 1.5 and 1.6 to confirm this is a regression?

          Show
          Christoph Reck added a comment - Interesting why this worked in 1.5... The api doc lists for Class FileUploadBase.SizeLimitExceededException: Methods inherited from class org.apache.commons.fileupload.FileUploadBase.SizeException getActualSize, getPermittedSize And Class FileUploadBase.SizeException has: Method Summary long getActualSize() long getPermittedSize() which means these are package private and therefore not allowed to be used externally. Can someone independently try it in 1.5 and 1.6 to confirm this is a regression?
          Hide
          Christoph Reck added a comment -

          I tested it with 1.5, where it works; whereas in 1.6.2 it fails - so this is a regression.

          It seems to be due to the protected class org.apache.commons.fileupload.FileUploadBase$SizeException - see:

          > javap -protected 'org.apache.commons.fileupload.FileUploadBase$SizeLimitExceededException'
          Compiled from "FileUploadBase.java"
          public class org.apache.commons.fileupload.FileUploadBase$SizeLimitExceededException extends org.apache.commons.fileupload.FileUploadBase$SizeException

          { public org.apache.commons.fileupload.FileUploadBase$SizeLimitExceededException(); public org.apache.commons.fileupload.FileUploadBase$SizeLimitExceededException(java.lang.String); public org.apache.commons.fileupload.FileUploadBase$SizeLimitExceededException(java.lang.String, long, long); }

          > javap -protected 'org.apache.commons.fileupload.FileUploadBase$SizeException'
          Compiled from "FileUploadBase.java"
          public abstract class org.apache.commons.fileupload.FileUploadBase$SizeException extends org.apache.commons.fileupload.FileUploadException

          { protected org.apache.commons.fileupload.FileUploadBase$SizeException(java.lang.String, long, long); public long getActualSize(); public long getPermittedSize(); }

          Seems to be duplicate of VELOCITY-579

          Show
          Christoph Reck added a comment - I tested it with 1.5, where it works; whereas in 1.6.2 it fails - so this is a regression. It seems to be due to the protected class org.apache.commons.fileupload.FileUploadBase$SizeException - see: > javap -protected 'org.apache.commons.fileupload.FileUploadBase$SizeLimitExceededException' Compiled from "FileUploadBase.java" public class org.apache.commons.fileupload.FileUploadBase$SizeLimitExceededException extends org.apache.commons.fileupload.FileUploadBase$SizeException { public org.apache.commons.fileupload.FileUploadBase$SizeLimitExceededException(); public org.apache.commons.fileupload.FileUploadBase$SizeLimitExceededException(java.lang.String); public org.apache.commons.fileupload.FileUploadBase$SizeLimitExceededException(java.lang.String, long, long); } > javap -protected 'org.apache.commons.fileupload.FileUploadBase$SizeException' Compiled from "FileUploadBase.java" public abstract class org.apache.commons.fileupload.FileUploadBase$SizeException extends org.apache.commons.fileupload.FileUploadException { protected org.apache.commons.fileupload.FileUploadBase$SizeException(java.lang.String, long, long); public long getActualSize(); public long getPermittedSize(); } Seems to be duplicate of VELOCITY-579
          Hide
          Nathan Bubna added a comment -

          Ick. Thanks for confirming, Christoph. I've got a little time today, so i'll see about setting up a testcase and fixing this.

          Show
          Nathan Bubna added a comment - Ick. Thanks for confirming, Christoph. I've got a little time today, so i'll see about setting up a testcase and fixing this.
          Hide
          Nathan Bubna added a comment -

          I've been thinking about this. The oft-stated intention of Velocity is to provide access to "public methods declared in public classes (or public interface)". The method in question is public and it is being called on an instance of a public subclass, but it is not actually declared in a public class. So, it was something of an undocumented, unintentional fluke that this worked previously. Such regressions are undesirable, but are arguably bug fixes. Certainly in the work for 1.6, we never thought about supporting "public methods declared or inherited by public classes".

          That said, i think the Commons-FileUpload folks are correct to have the SizeException class be protected and you are clearly justified to expect that a public method inherited by a public subclass should be callable via VTL. So it feels like more than a regression, this is exposing a bug in our "oft-stated intention".

          Though i haven't looked deeply at it, my instinct is that fixing this will either carry a significant performance penalty for ClassMap, require an even wider broadening of the methods support and/or undo other recent fixes. Unless further examination proves me wrong on that, it is not very likely that this will be un-regressed in 1.6.x. Right now, i'm guessing that fixing this will lead to a broadening of supported methods and thus be only fit for 1.7.

          Show
          Nathan Bubna added a comment - I've been thinking about this. The oft-stated intention of Velocity is to provide access to "public methods declared in public classes (or public interface)". The method in question is public and it is being called on an instance of a public subclass, but it is not actually declared in a public class. So, it was something of an undocumented, unintentional fluke that this worked previously. Such regressions are undesirable, but are arguably bug fixes. Certainly in the work for 1.6, we never thought about supporting "public methods declared or inherited by public classes". That said, i think the Commons-FileUpload folks are correct to have the SizeException class be protected and you are clearly justified to expect that a public method inherited by a public subclass should be callable via VTL. So it feels like more than a regression, this is exposing a bug in our "oft-stated intention". Though i haven't looked deeply at it, my instinct is that fixing this will either carry a significant performance penalty for ClassMap, require an even wider broadening of the methods support and/or undo other recent fixes. Unless further examination proves me wrong on that, it is not very likely that this will be un-regressed in 1.6.x. Right now, i'm guessing that fixing this will lead to a broadening of supported methods and thus be only fit for 1.7.
          Hide
          Nathan Bubna added a comment -

          Spent a little time trying to replicate the problem in a simple test case, with no success. Apparently i misunderstood the nature of the bug. Calling getDeclaredMethods on a public subclass of a protected and abstract superclass does return the superclass's public methods.

          Not sure what is causing the observed problem. Any ideas, Christoph?

          Show
          Nathan Bubna added a comment - Spent a little time trying to replicate the problem in a simple test case, with no success. Apparently i misunderstood the nature of the bug. Calling getDeclaredMethods on a public subclass of a protected and abstract superclass does return the superclass's public methods. Not sure what is causing the observed problem. Any ideas, Christoph?
          Hide
          David Esposito added a comment -

          Hi Guys,

          I was hoping for a resolution one way or the other on this. If it's not going to be "fixed", I will need to work around it ... If it is, when could I expect the fix to be included?

          Nathan, addressing the detail of "public methods declared in public classes" ... Why isn't it as simple as "public methods"? .. Why does the protection of the class have any relevance? From an end-user perspective, I would expect that I can invoke any 'getter' that I can from my Java code regardless of the inheritance scheme of the class ... In this case, I have the Javadocs and source code of the Commons-Fileupload classes to understand what the developers intended, but if it's a closed source library I'm using, I would have to decompile the binaries to figure out why my methods aren't being invoke from VTL.

          -Dave

          Show
          David Esposito added a comment - Hi Guys, I was hoping for a resolution one way or the other on this. If it's not going to be "fixed", I will need to work around it ... If it is, when could I expect the fix to be included? Nathan, addressing the detail of "public methods declared in public classes" ... Why isn't it as simple as "public methods"? .. Why does the protection of the class have any relevance? From an end-user perspective, I would expect that I can invoke any 'getter' that I can from my Java code regardless of the inheritance scheme of the class ... In this case, I have the Javadocs and source code of the Commons-Fileupload classes to understand what the developers intended, but if it's a closed source library I'm using, I would have to decompile the binaries to figure out why my methods aren't being invoke from VTL. -Dave
          Hide
          Nathan Bubna added a comment -

          I was unable to duplicate this in a test case, so fixing is rather problematic. I do, however, intend to move forward on VELOCITY-745 prior to a 1.7 release. That, i hope, would render this a non-problem.

          Show
          Nathan Bubna added a comment - I was unable to duplicate this in a test case, so fixing is rather problematic. I do, however, intend to move forward on VELOCITY-745 prior to a 1.7 release. That, i hope, would render this a non-problem.
          Hide
          Nathan Bubna added a comment -

          Actually, the test case does reproduce the problem, but only in JDK 1.5. It all works fine in JDK6.

          Show
          Nathan Bubna added a comment - Actually, the test case does reproduce the problem, but only in JDK 1.5. It all works fine in JDK6.
          Hide
          David Esposito added a comment -

          Just FYI, I definitely did repro this problem (my test program above, not the attached Test Case) on Sun JDK 1.6 (Sun JDK 1.6.0_16)

          Show
          David Esposito added a comment - Just FYI, I definitely did repro this problem (my test program above, not the attached Test Case) on Sun JDK 1.6 (Sun JDK 1.6.0_16)
          Hide
          Nathan Bubna added a comment -

          David, that is almost certainly because commons-fileupload 1.2 was compiled with JDK 1.5. The structure of the testcase code matches that of the commons-fileupload code in question. I would imagine you could fix your problem by recompiling fileupload with JDK 1.6, though i haven't tried that.

          Also, note the latest on VELOCITY-745, for a possible demonstration of why the allowed scope is "public methods declared in public classes". It appears that Method.invoke seems to care about the protection of the class in which the method was declared, at least in JDK 1.5, which is the target JDK for Velocity 1.7.

          Show
          Nathan Bubna added a comment - David, that is almost certainly because commons-fileupload 1.2 was compiled with JDK 1.5. The structure of the testcase code matches that of the commons-fileupload code in question. I would imagine you could fix your problem by recompiling fileupload with JDK 1.6, though i haven't tried that. Also, note the latest on VELOCITY-745 , for a possible demonstration of why the allowed scope is "public methods declared in public classes". It appears that Method.invoke seems to care about the protection of the class in which the method was declared, at least in JDK 1.5, which is the target JDK for Velocity 1.7.

            People

            • Assignee:
              Unassigned
              Reporter:
              David Esposito
            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Development