I think there is a small problem with isPushSupported() as it can be a bit
of a race inducing API:
if the API requires code like:
if (request.isPushSupported())
{
request.getPushBuilder()...... push()
}
Then there is a problem is the value of isPushSupported changes after
returning true - ie the getPushBuilder() or push() calls may throw ISE.
There are two cases that we have identified that isPushSupported() can
change value dynamically:
1. a client may at any time send a setting frame that disables support
for push. This is highly unlikely in reality, but may be part of some kind
of attack if server code is not expecting isPushSupported to change.
2. If resource A pushes B which attempts to push C, then our ability to
push C depends on the state of A. HTTP2 does not allow nested pushes, ie
we cannot send a push promise for C on B's stream. We can see that B
itself is a push and that it has a parent A. IFF stream A is still in an
appropriate state, then we can send a push_promise for C on A. But the
state of A can change asynchronously with the handling of B, so the ability
to push C is in a race!
So a simple fix for 2. is that we simply say that isPushSupported always
returns false for pushed resources - ie it will always return false when
handling B, so B will never attempt to push C. Frameworks will need to do
transitive dependencies so the handling of A will push both B and C.
I'm not sure how we should handle 1. It makes the throwing of an ISE
from getPushBuilder or push() after an isPushSupported very unlikely but
possible to induce. Perhaps a noop pushbuilder is actually better?
thoughts?
On 25 November 2015 at 17:09, Paul Benedict <pbenedict_at_apache.org> wrote:
> On Tue, Nov 24, 2015 at 3:28 PM, Greg Wilkins <gregw_at_webtide.com> wrote:
>
>> On 24 November 2015 at 02:16, Paul Benedict <pbenedict_at_apache.org> wrote:
>>
>>> I want to repeat a question I earlier sent:
>>> So what is the use case for the getters? Any examples? Because whoever
>>> is in charge of constructing of the PushBuilder has already determined
>>> and intended what is being pushed. I don't see why a builder needs to be
>>> queried for what's just been set.
>>>
>>
>> I don't have any concrete examples, however I can image a framework that
>> might pass a pushbuilder to several concerns, each to push their associated
>> resources.
>>
>
> Greg, it sounds to me like you're hinting at a PushListener. Maybe that
> would be a better design than passing around a PushBuilder?
>
>
>
>> Obviously it can be usable without getters, but then I see little harm in
>> providing them and some use (see below):
>>
>>> Also, after push(), it is documented that certain values will be cleared
>>> -- like path, etag, and lastModified. So I have some questions for the EG
>>> to ponder:
>>> 1) Should there be a boolean that represents if the builder is "clean"
>>> for a new build? A push would clean the builder.
>>> 2) Being reusable, should there be a count of how many pushes a builder
>>> performs?
>>>
>>
>> Both of these points are rather contrary to your "whoever is in charge
>> already knows" meme above:)
>>
>>
> You are right :-) I did allow myself an exception there because these
> questions were more about state than the data put in. But point granted.
>
> Cheers,
> Paul
>
--
Greg Wilkins <gregw@webtide.com> CTO http://webtide.com