ocaml-h2

ocaml-h2 copied to clipboard

This is a first attempt to replace the internal hpack library by ocaml-hpack.

Most of the changes are superficial, except for the header table size handling, which was not implemented before.

@314eter I was just waiting for the conformance tests to be run, and I was afraid this would happen, but there are some spec failures related to handling HPACK indexes and overflows.

Do you have a suggestion on how to fix these given how permissive your HPACK implementation is?

I already fixed them in 314eter/ocaml-hpack@8077a8530c41cbac0fcba0537c9da0ab1d5771d1, but the tests have to be run again.

Oh cool, let me trigger a new CI run

I expected h2spec 4.3.1 to fail, but it didn't because of a bug that is fixed in 314eter/ocaml-hpack#10.

@314eter I'm a little confused - so it will fail now if we update the dependency to include that new diff?

Yes, it will, because h2spec 4.3.1 interprets the rfc more strict than I do. It doesn't fail at the moment because it tests whether a table size update at the end of a header block is rejected. If it had tested with a table size update in the middle of a header block, it wouldn't have been rejected and the test would have failed.

I didn't merge the fix yet because another possible solution is to adapt to the h2spec interpretation, but that requires more work.

I was curious about the state of this PR -- wouldn't it be nice to just use the existing hpack package and not vendor a copy inside of this repository? I fail to understand what is missing from this PR to land.

@hannesm did you run into any issues that this PR fixes?

I remember I made some improvements, added fuzzing, and fixed a few bugs in ocaml-hpack after it was forked and vendored in ocaml-h2, and was still working on some other things. The bugs are probably still present in ocaml-h2. But I stopped working on hpack because everyone was using the h2 fork anyway.

If there is any interest, I can pick it up again.