RFR: 8328919: Add BodyHandlers / BodySubscribers methods to handle excessive server input [v10]
Volkan Yazici
vyazici at openjdk.org
Wed Jan 22 15:58:24 UTC 2025
On Wed, 22 Jan 2025 15:07:44 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>>> Instead, I need to verify against the cause
>>
>> Verifying against the cause is OK. Some of these tests, including this one are crafted based on what the implementation does. As long as we don't make the tests way too tied into the implementation (I believe this one doesn't).
>>
>>> This made me think, maybe HttpResponseInputStream::current should have started as follows:
>>>
>>> if (failed) throw new IOException(failed);
>>> if (closed) throw new IOException("closed");
>>>
>>> So that the exception message of failed will be preserved.
>>
>> In its current form in mainline, the exception message is still preserved in the underlying cause. Having said that, I think what you suggest for the `HttpResponseInputStream::current` implementation make sense since the current message says "closed" even if it may not be. If we do considering doing that change, we should do it in a separate issue.
>
>> @jaikiran, if the current resolution (i.e., https://github.com/openjdk/jdk/commit/70387718fabbd63ec94bb2b538e29931dce1fea2) addresses your concern, would you mind resolving this conversation, please?
>
> The change you did does address my review. I however don't see a way to resolve this conversation in the GitHub UI (I anyway don't resolve conversations because I find it hard to follow the discussion in these PRs since GitHub hides resolved conversations).
Got it, resolving the conversation – will discuss my `HttpResponseInputStream::current` proposal with the rest of the crew first.
<hr/>
I generally navigate through unresolved conversations using the `Conversations` drop down:

-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1925567770
More information about the net-dev
mailing list