831973741yy commented on code in PR #3028:
URL: https://github.com/apache/solr/pull/3028#discussion_r1912637510
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -784,7 +784,7 @@ private Request fillContentStream(
}
} else {
// application/x-www-form-urlencoded
- Fields fields = new Fields();
+ Fields fields = new Fields(true);
Iterator<String> iter = wparams.getParameterNamesIterator();
while (iter.hasNext()) {
Review Comment:
yep, the logic in this else block + convert function in FormRequestContent
is largely the same as wparams.toQueryString() except:
- leading question mark from wparams.toQueryString()
- wparams.toQueryString() uses hard coded utf-8 charset as oppose to the
FALLBACK_CHARSET passed to FormRequestContent
I guess we can replace it (I tried it, and all unit tests passed), but we
need to
1. put in some logic (if wparams.toQueryString().startwith("?") then
wparams.toQueryString().substring(1)) to handle the leading question mark from
wparams.toQueryString(). Although not likely, if the toQueryString function
changes the question mark logic in the future, we might have problem here.
2. assume FALLBACK_CHARSET remains utf-8 or pass a charset to
SolrParams.toQueryString function (not sure if adding a new toQueryString
function in SolrParams is justified for this special use case)
The current implementation with Fields has less dependencies stated above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
