[
https://issues.apache.org/jira/browse/SOLR-18343?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18112505#comment-18112505
]
David Smiley commented on SOLR-18343:
-------------------------------------
Reverting on 9.x; led to some mysteries I don't have time to solve on 9x.
Using POST exposed some issues. One is that
TestSolrCloudWithSecureImpersonation (related to deprecated auth stuff) needed
"user" in the URL query portion. I fixed this to ensure it got there. Next
I/Claude noticed
org.apache.solr.client.solrj.impl.SolrHttpRequestRetryHandler#handleAsIdempotent
wasn't activating because the path is /solr/admin/... and thus some admin
requests are actually getting retried when they shouldn't be. That uncovered
an issue... but then some tests became flaky (no retry). Shrug.
> SolrJ: require an explicit HTTP method on admin requests; stop defaulting to
> GET
> --------------------------------------------------------------------------------
>
> Key: SOLR-18343
> URL: https://issues.apache.org/jira/browse/SOLR-18343
> Project: Solr
> Issue Type: Improvement
> Components: SolrJ
> Reporter: David Smiley
> Priority: Major
> Labels: newdev, pull-request-available
> Fix For: 10.1
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> h2. Problem
> {{SolrRequest}} itself is fine — its single constructor requires a
> {{METHOD}}. The problem is one level down: the multi-action admin base
> classes hardcode GET, so none of their many subclasses ever states a verb.
> {code:java}
> // CollectionAdminRequest:87
> super(METHOD.GET, path, SolrRequestType.ADMIN);
> {code}
> The result is that CREATE, DELETE, SPLITSHARD, UNLOAD and the rest are all
> issued as HTTP GET. That is not defensible on HTTP grounds, and it misleads
> intermediaries — proxies and gateways are entitled to treat GET as safe, and
> some will retry it on a transient failure.
> h2. Proposal
> # *Require the verb.* Add {{METHOD}}-taking constructors to the admin base
> classes and deprecate the GET-defaulting ones, so each subclass is forced to
> think about it.
> # *Default macro handlers to POST.* For multi-action endpoints like
> {{/admin/collections}} and {{/admin/cores}}, POST is the safe default;
> actions known to be idempotent can specify GET explicitly.
> h3. Constructors that should change
> || Class || Line(s) || Note ||
> | {{CollectionAdminRequest}} | 87 | ~51 nested subclasses, 57 {{super(...)}}
> call sites |
> | {{CoreAdminRequest}} | 536, 540 | ~9 subclasses |
> | {{ConfigSetAdminRequest}} | 54, 58 | already has one POST override at
> {{:125}} for upload |
> h2. Scope
> Roughly three files. The ~66 subclass constructors that need a verb are all
> nested inside those same files, so it is a large diff in a small number of
> places rather than churn across the codebase. Repo-wide there are 255
> {{extends SolrRequest<}} declarations, but those already pass a verb
> explicitly and are unaffected.
> External impact is third-party subclasses of the three admin hierarchies,
> covered by a deprecation cycle.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
