facebook / zstd

Zstandard - Fast real-time compression algorithm
http://www.zstd.net
Other
23.8k stars 2.11k forks source link

HTTP "Content-Encoding" window size wrt. RFC 8878 #2713

Open klauspost opened 3 years ago

klauspost commented 3 years ago

Issue

It seems to me there is a need for RFC 8878 to specify a maximum window size for using "zstd" Content-Encoding. At least 8MB is recommended as max window size for the decoder, but is not a requirement.

The main issue I see with wide adoption is the risk of having to reject data because of the window size chosen by the encoder.

Describe the solution you'd like

A clear and concise limit on what the max accepted windows size for is HTTP transfers. Then the client has an easy choice, will it accept content with a window up to the limit. If not, it will not add "Accept-Encoding": "zstd".

Describe alternatives you've considered

The alternative is to specify nothing.

In practice what I could see happen is that each client sets its own limit, 64K, 1M, 8MB. The server would either have to just use the smallest accepted window limit (likely) or do client sniffing (unlikely to happen).

Additional context

In my personal opinion even 1MB is rather high. This is considering that browsers often have multiple connections open concurrently to multiple sites.

That said, I can understand you would make it 8MB as per the existing recommendation.

This limit should only apply when using zstd as transfer encoding.

felixhandte commented 3 years ago

Specifying a hard limit on the window size for HTTP transfers is not a good solution, IMHO. A global limit that is conservative enough to be of use to resource-constrained endpoints will be harmfully restrictive for use cases which (by prior arrangement or expectation) transport large zstd-compressed objects.

I would much rather have standardized an option on the Accept-Encoding item ("Accept-Encoding: zstd;w=20"). This would provide a standard mechanism for recipients to make their limitations known to senders. Unfortunately we did not do so, and are not planning to revise the RFC again anytime soon.

Frankly, we might encourage folks to use such a mechanism anyways. If we were to specify a w option in the future, I can only imagine that w=N would mean that the maximum window size the recipient is prepared to handle is 2**N bytes. So ad hoc use should be forwards-compatible...

Otherwise, in the absence of a coordination mechanism, I think implementations should just follow Postel's Principle: "be conservative in what you send, be liberal in what you accept".

I'm curious to hear more about the experiences you've seen where this is a problem.

klauspost commented 3 years ago

@felixhandte I don't disagree with anything you say, but I think you should be more proactive in your approach. Experience shows that if there is a problem an encoding is rather dropped than having to wait for server implementations to be corrected/catch up.

A w=N sounds tempting since it allows the client to specify exactly what it wants. This would not be usable for static content, unless a really conservative size is used. It would also mean that static content would need to keep track of N, or even parse the frame header if compressed externally.

felixhandte commented 3 years ago

I spoke to a couple people and it sounds like this would be relatively straightforward to specify. I've just spun up a thread on the IETF httpbis mailing list to discuss. Your input there would be appreciated!

Edit: link to thread.

ricea commented 1 year ago

This was discussed in the WebPerf working group at TPAC 2023. Our conclusion was to file an errata against the RFC to request an explicit definition of window size for the "zstd" content-encoding.

ricea commented 10 months ago

We now have an example of this causing incompatibility in the wild: https://bugs.chromium.org/p/chromium/issues/detail?id=1520682

In a web browser, we can't necessarily trust the servers we connect to, so we can't allocate 2GB of RAM just because the server says so. Being liberal in what we accept isn't going to work for us.

Unless someone has a better idea, I will file an errata against RFC 8478.

felixhandte commented 10 months ago

Hi @ricea,

Yeah, since it turned out that the accept-encoding header does not permit sub-options other than q=, we would have to introduce a whole new header to do any kind of negotiation, which is too complicated and likely too expensive in header bytes to justify.

I see a few options that don't involve negotiation, in increasing order of complexity/specificity:

  1. Errata the 8MB recommendation to a flat requirement.
  2. Switch to requiring <=8MB unless the recipient is known to support a larger window by some external/unspecified means.
  3. Require <=8MB for browsers specifically, but leave non-browser use cases as-is.
  4. Require <=8MB specifically for Chrome (and we can accumulate additional user agents as they ship support and pick how to limit).

I'm most attracted to option 3, since it's still a fairly simple prescription, but doesn't block the non-browser use case of shipping huge tarballs and stuff around via HTTP where big windows are valuable.

What do you think about the practicality of this sort of more nuanced rule?

(P.S., we'll want to file it against RFC8878, the updated spec.)

klauspost commented 10 months ago

@felixhandte Option 1.

Anything else is just overengineering and will just cause confusion down the line. If client cannot handle that they shouldn't request it - simple. But that is just my humble, outsider opinion.

Cyan4973 commented 10 months ago

There are legitimate use cases (outside of the web domain) for which long Windows is an important capability. For example, when transmitting a delta vs a reference resource.

Note that protecting the client (receiver) from unreasonable Window Size requirements is already possible when using the reference library (and I believe chrome uses it), so this potential vector of attack is already well covered.

After that, deciding which limit is suitable depends on the domain. For example, there are networks of small iot sensors out there which only permit very small Window sizes (KB range) due to local hardware limitations.

Setting a limit adjusted for the web domain is a sensible decision. RFC8878 suggests an 8 MB Window Size, and this is certainly negotiable if web folks feel strongly about a different limit. At this point, in the absence of counter-proposals, it still looks like a reasonable one to me.

We could possibly strengthen the wording in the RFC to make it more "the" established limit for the web, rather than a suggested one. Such a change of wording would be an outcome of this discussion.

nidhijaju commented 10 months ago

Option 3 sounds attractive, but Option 2 currently sounds better to me because it provides more predictability and consistency across all use cases, unless otherwise negotiated via an alternate mechanism. If we want to cater to different domains, each one can negotiate a limit suitable for its needs.

Chrome does use the ZSTD_d_windowLogMax parameter, and although the vector of attack is mitigated, it still results in decoding issues that are inconsistent with other clients, causing confusion and hard-to-debug incompatibilities. Having an agreed-upon limit that everyone used (instead of the suggestion that some people may not follow) would allow better interoperability.

felixhandte commented 8 months ago

Hey all, here's what we came up with:

In RFC 8878 Section 3.1.1.1.2. Window Descriptor:

   For improved interoperability, it's recommended for decoders to
   support values of Window_Size up to 8 MB and for encoders not to
   generate frames requiring a Window_Size larger than 8 MB.  It's
   merely a recommendation though, and decoders are free to support
   higher or lower limits, depending on local limitations.

would become

   For improved interoperability, it's recommended for decoders to
   support values of Window_Size up to 8 MB. It's just a
   recommendation, decoders are free to support higher or lower
   limits, depending on local constraints. In the absence of other
   negotiation or specification, encoders should refrain from
   generating frames requiring a Window_Size larger than 8 MB. In
   the specific case of encoding web content intended for
   consumption by web browsers, encoders must not generate frames
   with a Window_Size larger than 8 MB.

What do you think?

klauspost commented 8 months ago

It's just a recommendation, decoders are free to support higher or lower limits, depending on local constraints.

No. Decoders should always support 8MB when requesting zstd encoding in a browser. Otherwise they should not request zstd. Having a vague standard is worse than no standard IMO.

Cyan4973 commented 8 months ago

The deployment scope of Zstandard is not limited to the web. There are already infrastructures and hardware employing Zstandard with much lower window size limits than 8 MB. And that's fine. They just can't expect to grab some random stuff over Internet and be able to decode. This is generally okay, because such deployments tend to operate in semi-private environments, and interactions with the external world are limited and well controlled. The IETF RFC 8878 scope is general, it includes those environments.

Enforcing 8 MB as an official limit, for both the encoder and decoder, should be specified as limited to a specific domain, HTTP Content-Encoding in this case.

felixhandte commented 8 months ago

@klauspost, I see your point. I can also add decoder requirements specific to the web. Something like "Decoders handling web content must support a Window_Size of at least 8 MB."

Does that address your concern?

nidhijaju commented 8 months ago

I'd been working on an internet draft for the window size issue for zstd Content Encoding, as it seemed better than an errata especially given we're trying to change the requirements of the document, and finally got a chance to push it to https://github.com/nidhijaju/draft-zstd-window-size. Hopefully it addresses @klauspost's concern as well about making the 8MB requirement more explicit for HTTP Content Encoding contexts. Please feel free to open any issues on the repo.

I also started a thread on the IETF httpbis mailing list at https://lists.w3.org/Archives/Public/ietf-http-wg/2024JanMar/0153.html, so please feel free to leave any comments there. Thank you!

klauspost commented 8 months ago

Love it!