Thanks for opening the issue Uwe! I agree that option #1 is the best long term fix (although for the moment I’ve wrapped the relevant ES code in a doPrivileged block which solves the immediate problem).
> Is Elasticsearch now using the Analysis Factory framework instead of their > own factories? No, we still have our own factory implementations. In fact, looking more closely at this, I don’t think we need to call these reload SPI methods at all and I can probably remove it entirely :) Still, it found a possible bug! > On 13 Sep 2021, at 12:48, Uwe Schindler <u...@thetaphi.de> wrote: > > See https://issues.apache.org/jira/browse/LUCENE-10101 > > ----- > Uwe Schindler > Achterdiek 19, D-28357 Bremen > https://www.thetaphi.de > eMail: u...@thetaphi.de > >> -----Original Message----- >> From: Uwe Schindler <u...@thetaphi.de> >> Sent: Monday, September 13, 2021 9:59 AM >> To: dev@lucene.apache.org >> Subject: RE: getField vs getDeclaredField in analysis SPI >> >> Hi Alan, >> >> I will open an issue about this! >> >> Uwe >> >> ----- >> Uwe Schindler >> Achterdiek 19, D-28357 Bremen >> https://www.thetaphi.de >> eMail: u...@thetaphi.de >> >>> -----Original Message----- >>> From: Uwe Schindler <u...@thetaphi.de> >>> Sent: Monday, September 6, 2021 4:57 PM >>> To: dev@lucene.apache.org >>> Subject: RE: getField vs getDeclaredField in analysis SPI >>> >>> Hi Alan, >>> >>> Would you open issue, I will take it!? >>> >>> 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. >>> >>> Uwe >>> >>> ----- >>> Uwe Schindler >>> Achterdiek 19, D-28357 Bremen >>> https://www.thetaphi.de >>> eMail: u...@thetaphi.de >>> >>>> -----Original Message----- >>>> From: Alan Woodward <romseyg...@gmail.com> >>>> Sent: Monday, September 6, 2021 3:48 PM >>>> To: dev@lucene.apache.org >>>> Subject: Re: getField vs getDeclaredField in analysis SPI >>>> >>>> Thanks Uwe! >>>> >>>>> On 6 Sep 2021, at 13:11, Uwe Schindler <u...@thetaphi.de> wrote: >>>>> >>>>> Hi Alan, >>>>> >>>>>> 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: >>>>> - Change to getField(), but in the if statement below check the actual >> class: >>>> (field.getDeclaringClass()==service) (see https://github.com/apache/lucene- >>>> solr/pull/1360/files#diff- >>>> >>> >> 6a65d91199a18bc4ee2d00a1e9dc283aedc4134846e0d7aafdc484f8263e250bR >>>> 159-R162) >>>>> - Wrap with doPrivileged in Lucene code. As far as I remember Lucene >> needs >>>> the permission anyways. With doPrivileged you would delegate >> responsibility. >>>>> >>>>> I'd open a JIRA issue, I can fix this. It only affects Lucene 9.0. >>>>> >>>>>> understand it it’s looking for a NAME static field on the class in >>>>>> question, >>>> which >>>>>> should always be public. I’m in the process of upgrading elasticsearch to >>> use >>>> a >>>>>> lucene 9 snapshot, and this change means that I need to wrap SPI >>> reloading >>>>>> code in doPrivileged() blocks, which is a bit of a pain. >>>>> >>>>> Thansk for doing this. Is Elasticsearch now using the Analysis Factory >>>> framework instead of their own factories? >>>>> >>>>>> Thanks, Alan >>>>> >>>>> No problem, >>>>> Uwe >>>>> >>>>> >>>>> >>>>> --------------------------------------------------------------------- >>>>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org >>>>> For additional commands, e-mail: dev-h...@lucene.apache.org >>>>> >>>> >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org >>>> For additional commands, e-mail: dev-h...@lucene.apache.org >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org >>> For additional commands, e-mail: dev-h...@lucene.apache.org >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org >> For additional commands, e-mail: dev-h...@lucene.apache.org > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: dev-h...@lucene.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
