Skip to content

SOLR-15418: V2 API: Fix GET to /select and others#134

Merged
dsmiley merged 3 commits intoapache:mainfrom
dsmiley:SOLR-15418-dontAddEmptyStreams
May 24, 2021
Merged

SOLR-15418: V2 API: Fix GET to /select and others#134
dsmiley merged 3 commits intoapache:mainfrom
dsmiley:SOLR-15418-dontAddEmptyStreams

Conversation

@dsmiley
Copy link
Contributor

@dsmiley dsmiley commented May 19, 2021

Indirectly fixed by not adding ContentStreams that are empty.
https://issues.apache.org/jira/browse/SOLR-15418

Indirectly fixed by not adding ContentStreams that are empty.
@dsmiley dsmiley requested a review from noblepaul May 19, 2021 19:07
@dsmiley
Copy link
Contributor Author

dsmiley commented May 19, 2021

I tried to avoid the PushBackInputStream thing but in the end I found it necessary. At least it will rarely be used, I think. I did ensure this code actually works by temporarily removing the short-circuit conditions and then running the test.
I could add a method==GET check as well since I think it's fundamentally invalid?

FYI this is called by StandardRequestParsers.parseParamsAndFillStreams. I don't like the way that method makes assumptions about paths for /schema and v2 API which seem to be concerns that should be elsewhere like HttpSolrCall (if necessary) or perhaps give handlers the opportunity to get involved in this (if necessary).

@dsmiley dsmiley requested a review from gerlowskija May 19, 2021 19:17
@dsmiley dsmiley merged commit aeb617d into apache:main May 24, 2021
@dsmiley dsmiley deleted the SOLR-15418-dontAddEmptyStreams branch May 24, 2021 16:08
madrob pushed a commit to madrob/solr that referenced this pull request May 25, 2021
Indirectly fixed by not adding ContentStreams that are empty.
epugh pushed a commit to epugh/solr that referenced this pull request Oct 22, 2021
Indirectly fixed by not adding ContentStreams that are empty.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant