Skip to content

http2: correct behaviour for enablePush unpack

Rodrigo Muino Tomonari requested to merge github/fork/apapirovski/patch-http2-unpacksettings into master

So, as per the spec, the only valid values for enablePush should be 0 or 1. nghttp2 already handles this correctly but the JS unpacker exposed to the end-user doesn't. That said, I'm not sure as to the desired behaviour:

  1. Set value to the actual value, validate it's within range (if validation requested) and finally set to Boolean (even if it's not 0 or 1, as long as they didn't request validation which would throw).

  2. Set value to the actual value, validate it's within range (if validation requested) and only set to Boolean if it's 0 or 1, otherwise leave as the actual value for the end-user to handle.

I think we should follow the spec here, at least to the extent that we shouldn't just cast everything to Boolean when validating. If validation is requested then it clearly needs to throw if it's not 0 or 1 (it wasn't doing this before).

But not sure what we actually want to output to the end-user if it's not 0 or 1 and no validation was requested.

And here's the spec btw:

SETTINGS_ENABLE_PUSH (0x2): This setting can be used to disable server push (Section 8.2). An endpoint MUST NOT send a PUSH_PROMISE frame if it receives this parameter set to a value of 0. An endpoint that has both set this parameter to 0 and had it acknowledged MUST treat the receipt of a PUSH_PROMISE frame as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.

The initial value is 1, which indicates that server push is permitted. Any value other than 0 or 1 MUST be treated as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

http2, test

Merge request reports