lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Smiley (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (SOLR-11501) Depending on the parser, QParserPlugin should not parse local-params
Date Fri, 20 Oct 2017 04:39:00 GMT

     [ https://issues.apache.org/jira/browse/SOLR-11501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

David Smiley updated SOLR-11501:
--------------------------------
    Attachment: SOLR_11501_limit_local_params_parsing.patch

Here's a patch. It has a bunch of nocommits but it's very close.  {{ant test-core}} passes
with a couple minor test adjustments. It essentially has two parts:

*QParser.getParser*: now only parses local-params when the provided defaultParser is "lucene"
or "func".  Across our codebase, those were the only two cases where the callers might want
to permit the user to change the query parser.
* {{ChildFieldValueSourceParser}} had set the default to "parent" but it's pointless; might
as well use null/"lucene" since there has to be local-params to set.
* Some users of "hl.q" param might need to set "hl.qparser=lucene" now if they want to use
local-params in "hl.q". This could happen if defType=edismax.  I think this new setting is
safer.
* One of the "query" (nested subquery) tests used a defType of "query" yet used local-params.
This seems wrong?  I don't think NestedQParserPlugin needs to be an exception.  BTW I suspect
the utility of this thing is obsoleted these days.
* test in SOLR-11512 (infinite loop) included and passes! (caught exception, triggered by
the recursion detector)

*SolrQueryParserBase*: now has a {{allowSubQueryParsing}} option. The parsing of local-params
and \_query\_ and \_func\_ is guarded by this.
* {{LuceneQParser}} sets it to true.  Perhaps we don't need yet another param toggle here.
* {{ExtendedDismaxQParser}}: What I've done is leverage the "uf" param to toggle this to see
when it contains \_query\_.  Additionally, I've modified the defaults so that \_query\_ is
negated by default; it needs to be explicitly included now.
* {{DisMaxQParser}} added a simple toggle param.  Perhaps it could be made to read Edismax's
"uf"?  Or we finally retire this sucker ;-)   -- so... maybe add no toggle and just force
you to upgrade to edismax if you want control of this.

[~yseeley@gmail.com] I'm really curious what you think.

> Depending on the parser, QParserPlugin 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message