[ 
https://issues.apache.org/jira/browse/SOLR-11501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16217550#comment-16217550
 ] 

David Smiley commented on SOLR-11501:
-------------------------------------

bq. what about a specification of what parsers are allowed/disallowed?

Sounds good to me.  Could be added in a separate issue.  Here I can try to 
structure the code to make the ability to parse it be more dynamic rather than 
binary.

bq. We could even make this do double-duty... be able to turn off lucene parser 
builtins as well (fuzzy queries, etc?)

Interesting idea.  There's a namespace issue though, unless we prefix/suffix 
one or the other to mitigate that.  

bq. For dismax, it seems like bug this ever parsed/used local params, the 
escaping should be enhanced so this doesn't happen. Both dismax and edismax 
need to not throw exceptions when encountering localParams as well.

I don't think Dismax (or eDismax) needs to be modified for new escaping rules.  
Both parsers already catch exceptions from the parser and relax it in a 
simplified manner.  I can add some tests to make this fact more clear.

bq. As far as this patch... I don't know at this point in time. It's hard to 
figure out what might break or what behavior is undesirable. 

The patch is fairly small and with it applied, all tests pass :-)  Of course 
some user is bound to use it in a way that will no longer be supported but it's 
telling the changes in our codebase were so limited.

bq. Need to look at all of the places that use a defType other than lucene and 
func.

FWIW I did.  Specifically, the method to find the callers of is 
{{getParser(String qstr, String defaultParser, SolrQueryRequest req)}}.  In 
solr-core there are 10 usages, and analytics module 1.  Tests add 23 more but 
they are not pertinent.

bq. Why not do it the other way: disable for dismax, optionally disable for 
extended. That would seem to make it much easier to reason about.

I'd be happy to outright disable subquery parsing (with no option/toggle) on 
dismax!  I spoke of this in my comment; I'll modify the patch accordingly.  And 
it's optional for edismax (toggle'able via {{uf}}); maybe I'm misunderstanding 
you?

> Depending on the parser, QParser should not parse local-params
> --------------------------------------------------------------
>
>                 Key: SOLR-11501
>                 URL: https://issues.apache.org/jira/browse/SOLR-11501
>             Project: Solr
>          Issue Type: Improvement
>      Security Level: Public(Default Security Level. Issues are Public) 
>          Components: query parsers
>            Reporter: David Smiley
>            Assignee: David Smiley
>         Attachments: SOLR_11501_limit_local_params_parsing.patch
>
>
> Solr should not parse local-params (and thus be able to switch the query 
> parser) in certain circumstances.  _Perhaps_ it is when the QParser.getParser 
> is passed "lucene" for the {{defaultParser}}?  This particular approach is 
> just a straw-man; I suspect certain valid embedded queries could no longer 
> work if this is done incorrectly.  Whatever the solution, I don't think we 
> should assume 'q' is special, as it's valid and useful to build up queries 
> containing user input in other ways, e.g. {{q= +field:value +\{!dismax 
> v=$qq\}&qq=user input}}      and we want to protect the user input there 
> similarly from unwelcome query parsing switching.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to