Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 9.0
    • 9.0
    • None
    • New

    Description

      Discussion between Uwe Schindler and Alan Woodward:

      > LUCENE-9281 moved the `lookupSPIName` method from
      > AbstractAnalysisFactory to AnalysisSPILoader; the method is mostly the same,
      > but one line has been changed from Class.getField() to Class.getDeclaredField().
      > This can fall foul of the Security Manager, which wants a higher level of
      > permission for getDeclaredField. Was this an intentional change? As I

      This was intentional because the previous code wasn't fully correct, because I had some safety check in mind: The main reason for the getDeclaredField() is to lookup the field only in this class; while getField() also looks into superclasses. E.g. if the superclass has a NAME field because of a programming error it would pick that up, which would be wrong. When investigating other implementations using "named" lookups out there (even in JDK), they used getDeclaredField() when accessing a static member.

      There are 2 solutions:

      Maybe also post your opinion about think fix #1 or fix #2 is better. I tend to go for fix #1. getDeclaredField() should theoretically be faster, but that won't matter here: If it goes the slow path (going up to superclass) it will fail anyways and that's the exceptional case. A correct factory should have a NAME field and its lookup is fast and the additional check introduced for the class is cheap.

      This is the issue to implement one of the solutions, preferably #1

      Attachments

        Issue Links

          Activity

            People

              uschindler Uwe Schindler
              uschindler Uwe Schindler
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 20m
                  20m

                  Slack

                    Issue deployment