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

David Smiley commented on SOLR-18345:
-------------------------------------

Encoding values in Jira is such a pain; I'm not sure how interpret what was 
intended.  A clearer way to communicate might be a code snippet of a test.  
Please see the PR (now merged) and its very clear tests.  Maybe I should have 
credited you as the inadequacies were identified by you ~5 years ago and you 
had an improved variant, and then I iterated on it to be more thorough (with 
AI).  Any way, I believe the requirement of this method should be to encode the 
provided input String of this method so that the Solr receives it and uses a 
parser that yields the same output.  Any exceptions to this are problematic 
{_}for reasons you stated{_}.  Are you arguing *for* special-cases?  e.g. if 
the value contains a backslash then we assume it has special meaning (what?); 
and why do that?  

There is one global special case across all params that I think is pretty 
terrible and bit me years ago at my last job.  – the super secret magic 
parameter expandMacros to disable ampersand processing that can occur almost 
anywhere, silently replacing if another param just so happens to exist but 
otherwise doing nothing.  Ouch!

> ClientUtils.encodeLocalParamVal() can produces lossy/invalid encodings
> ----------------------------------------------------------------------
>
>                 Key: SOLR-18345
>                 URL: https://issues.apache.org/jira/browse/SOLR-18345
>             Project: Solr
>          Issue Type: Bug
>            Reporter: David Smiley
>            Assignee: David Smiley
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> ClientUtils.encodeLocalParamVal(String) safely embeds an arbitrary literal
> value into a Solr local-params string (e.g. \{!key=<value>\}, for consumption
> by QueryParsing#parseLocalParams / StrParser. It had two related bugs:
> 1. It escaped an embedded single quote (') but not an embedded backslash (\).
>    Since StrParser#getQuotedString treats '\' as the start of an escape
>    sequence, an unescaped backslash in the value is misinterpreted on
>    decode (e.g. a literal "\n" in the value becomes an actual newline).
> 2. It failed to quote a value whose first character is itself a quote
>    character ('or "), even when no other quoting trigger (whitespace or '}')
>    was present. QueryParsing#parseLocalParams treats a quote character
>    immediately after '=' as the start of a quoted value, so such values were
>    misparsed (e.g. "''" round-tripped to "").
> Both cause encoded values to not round-trip correctly, so callers building
> local-params strings from arbitrary field values/keys could produce
> corrupted or unparseable queries.
> Fix: escape backslashes in addition to single quotes, and force quoting
> whenever the value starts with a quote character.
> _(written by AI)_



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

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

Reply via email to