Buffer dyno status line up to a specific limit about vegur HOT 9 CLOSED

I have a fix for it that carries the BUFFER_MAX size. Maybe that's too small.

I believe we might have a similar bug when reading headers: https://github.com/heroku/vegur/blob/master/src/vegur_client.erl#L388-L455 We should limit impose a limit on each pair.

from vegur.

Good catch!

from vegur.

A protection similar to what is enabled by this is what we'd probably need there, with a large value to prevent breaking anything.

from vegur.

Yeah. I did a small test that enforced the same limit but had a number of tests failing and decided to wait with it until the vegur_logging branch gets merged.

from vegur.

@ferd I looked into Pull Request #37 and it does not protect us agains a bad dyno sending a single header pair that might blow our memory usage. Since we split on \r\n I could send a header of the following format:

HeaderKeyThatGoesOnForever

or

HeaderKey: InfinityLongString

and we would buffer it.

from vegur.

Good point. Further protection needs to be added there. The one I've added for cookies turns out to only block out possibly fair usage that would have killed the requests coming back into our system.

That should possibly be checked at https://github.com/heroku/vegur/blob/88890bb7dab96157d9b09bf6538275b2d5a35c4e/src/vegur_client.erl#L455-461 or maybe at the top of the loop itself, and we'll need to add tests. I'll try to do something about it this morning while I wait for reviews of benchmarks.

from vegur.

Limits put from the dyno in current stack:

• header name length: 1000 characters
• total header size/value size: >1MB, I hit gateway timeouts before 502s.

For this reason, I'm thinking we should set a large header value possible (512kb so we break nothing), keep the 1000 header length, and enforce an artificially low value for cookie values themselves to prevent DoS.

from vegur.

+1 on that design.

from vegur.

Handled in pull req #40

from vegur.