Skip to content
This repository has been archived by the owner on Mar 8, 2021. It is now read-only.

Header names #34

Open
mnot opened this issue Aug 8, 2019 · 18 comments
Open

Header names #34

mnot opened this issue Aug 8, 2019 · 18 comments
Labels
help wanted Extra attention is needed

Comments

@mnot
Copy link
Collaborator

mnot commented Aug 8, 2019

RateLimit-Limit is not a great name... suggest making them more concise.

@whiskeysierra
Copy link
Collaborator

I believe the big selling point is the widespread use of those names already.

@mnot
Copy link
Collaborator Author

mnot commented Aug 8, 2019

Using the same header names will cause problems unless there's already very good interop across different implementations, and we don't change anything.

@whiskeysierra
Copy link
Collaborator

From what I can see the current draft is compatible with some implementations out there. Obviously not all of them. But proposing yet another set of headers would just create this situation, wouldn't it: https://xkcd.com/927/?

@mnot
Copy link
Collaborator Author

mnot commented Aug 8, 2019

If you want to take a cynical view, sure. The other uses aren't standard, however; they're one-off, site-specific semantics.

Regardless, this is something we can discuss if the draft is adopted.

@whiskeysierra
Copy link
Collaborator

Fair enough. Do you have any names in mind?

@ioggstream
Copy link
Owner

ioggstream commented Aug 8, 2019

Regardless, this is something we can discuss if the draft is adopted.

Agree. The semantic now is core.

the current draft is compatible with some implementations out there

Yes, that was the purpose of the intro. See #15.

@ioggstream ioggstream added the help wanted Extra attention is needed label Aug 19, 2019
@ioquatix
Copy link

It's uncommon to have concatenated words without a hyphen in HTTP headers. e.g. Content-Type. I can't think of many standardised headers which have something like FooBar-Baz. To be consistent, it should be Foo-Bar-Baz.

That being said, I actually agree, if there is legacy usage, using the same name could be risky, and maybe it's better just to have a clean cut.

The key point would be... maybe acknowledging existing usage so that it's clear what's going on to those implementing the standard.

Regarding headers, I'd carefully consider mutabilty w.r.t. hpack.

If there are fields that change infrequently, don't bundle them with fields that change every other request.

Because HPACK will compress those unchanging fields down to 1-2 bytes.

Additionally, on the server, it's nice to be able to tell HPACK to ignore some fields that we know would be changing every request, because that way it won't get added to the dynamic dictionary and take up useful space.

So, I don't know specifically how it should be structured, but I can think of at least one way:

  • A header which represents all the static details of the rate limiting configuration. Ideally unchanging for the duration of the persistent connection.
  • A header which represents the current state of the rate limiting w.r.t. the current request and/or connection. Expected to change each request.

Regarding how to name this, I guess the first step would be looking at existing standardised headers.

Finally, one additional detail which might be useful, if it's possible just to have one header... you can still send dynamic data by splitting it into multiple headers.

Rate-Limit: <per-connection stuff>
Rate-Limit: <per-request stuff>

You could potentially look at how the new Server-Timing header which can bundle multiple records containing different information into a single header.

@ioggstream
Copy link
Owner

Thanks @ioquatix for your detailed analysis!

hyphens and single-header

To be consistent, it should be Foo-Bar-Baz.

I'd see RateLimit more like ETag :)

if it's possible just to have one header
send dynamic data by splitting it into multiple headers

we thought about it, but I strongly agree with @whiskeysierra #34 (comment)

hpack & performance

If there are fields that change infrequently,
don't bundle them with fields that change every other request.

good catch, but iiuc hpack's benefits mostly for ingress traffic
and those are response headers.

on the server, it's nice to be able to tell HPACK to ignore [frequently changing headers]
, because [they] won't get added to the dynamic dictionary [..].

That's interesting: can you PR a FAQ on that? @ioquatix

A header which represents all the static details of the rate limiting configuration

Only the server knows whether the RateLimit headers are static or not: see this example.

https://ioggstream.github.io/draft-polli-ratelimit-headers/draft-polli-ratelimit-headers.html#rfc.section.6.2.4

Ideally unchanging for the duration of the persistent connection.

We avoided mentioning connections as that's outside our intended semantic.
afaik, in http/2 a per-user ratelimit policy, different users may share the
same connection and receiving different ratelimit headers.

@ioquatix
Copy link

Compatibility with existing headers is useful but I don't think it's a necessity, especially if you can make a big reduction in overhead.

Regarding HPACK, it applies to both request and response headers equally.

Here is an example regarding the compression strategy:

#!/usr/bin/env ruby

require 'protocol/hpack'

def current_proposal
	buffer = String.new.b
	compressor = Protocol::HPACK::Compressor.new(buffer)
	
	10.times do |i|
		compressor.encode([
			["RateLimit-Limit", "10, 10;w=1, 50;w=60, 1000;w=3600, 5000;w=86400"],
			["RateLimit-Remaining", "#{60 - i}"],
			["RateLimit-Reset", "#{60 - i}"],
		])
		
		puts "Cumulative total after #{i+1} request(s): #{buffer.bytesize}B"
	end
	
	puts "Dynamic table: #{compressor.context.current_table_size}B, #{buffer.bytesize}B across the wire."
end

def another_option
	buffer = String.new.b
	compressor = Protocol::HPACK::Compressor.new(buffer)
	
	10.times do |i|
		compressor.encode([
			["Rate-Quota", "10, 10;w=1, 50;w=60, 1000;w=3600, 5000;w=86400"],
			["Rate-Limit", "#{60 - i}/#{60 - i}"]
		])
		
		puts "Cumulative total after #{i+1} request(s): #{buffer.bytesize}B"
	end
	
	puts "Dynamic table: #{compressor.context.current_table_size}B, #{buffer.bytesize}B across the wire."
end

current_proposal
another_option

Output:

Cumulative total after 1 requests: 84B
Cumulative total after 2 requests: 95B
Cumulative total after 3 requests: 106B
Cumulative total after 4 requests: 117B
Cumulative total after 5 requests: 128B
Cumulative total after 6 requests: 139B
Cumulative total after 7 requests: 150B
Cumulative total after 8 requests: 161B
Cumulative total after 9 requests: 172B
Cumulative total after 10 requests: 183B
Dynamic table: 1113B, 183B across the wire.
Cumulative total after 1 requests: 59B
Cumulative total after 2 requests: 66B
Cumulative total after 3 requests: 73B
Cumulative total after 4 requests: 80B
Cumulative total after 5 requests: 87B
Cumulative total after 6 requests: 94B
Cumulative total after 7 requests: 101B
Cumulative total after 8 requests: 108B
Cumulative total after 9 requests: 115B
Cumulative total after 10 requests: 122B
Dynamic table: 558B, 122B across the wire.

@ioggstream
Copy link
Owner

Regarding HPACK, it applies to both request and response headers equally.

True, see the gain gap between requests and response though https://blog.cloudflare.com/hpack-the-silent-killer-feature-of-http-2/

@ioquatix
Copy link

Great article, but that is relating to general requests/responses not specifically how headers are compressed or decompressed - it doesn't make any difference if it's request or response, the exact same logic is applied.

unleashed added a commit that referenced this issue Oct 30, 2019
Address some faq for header names. See #34
@ioquatix
Copy link

ioquatix commented Nov 14, 2019

I was just thinking there is at least one example where the adopted headers didn't match the unofficial headers: X-Forwarded-*.

https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/

So, there is at least a precedent for choosing better names than what has been the defacto standard.

I personally believe that there is a balance, but for me it's slightly in favour of the almost unlimited code that has yet to be written, rather than the legacy systems which will mostly be discarded.

@ioquatix
Copy link

Also, regarding the FAQ entry, I don't think I was advocating for a single header - it's fine to have one header for static content and one header for dynamic content, e.g. as proposed Rate-Limit vs Rate-Quota.

@ioggstream
Copy link
Owner

ioggstream commented Nov 15, 2019

regarding the FAQ entry, I don't think I was advocating for a single header

Before writing the draft I considered the single-header stuff we thought too when first dealing with the subject (), so I thought it was worth mentioning it in the FAQ :)

about X-Forwarded ...

I know, and one of the open issues we have in the Italian API guidelines is the following:

  • we have ~20k agencies including cities with related services that should interoperate,
    each one with their service provider which may be on-premise or cloud gateways
  • we aim at using the new and standard forwarded header, which is significantly
    better respect to the not a standard x-forwarded-*
  • OTOH x-forwarded-* is widely implemented in a coherent
    way by various implementors so people tend to use it.

As of now - even if we would enforce forwarded - agencies had still to support x-forwarded-*.

In this case, which is much simpler that forwarded we decided then to avoid hassles and enable a straightforward transition between the old and new model.

@mnot
Copy link
Collaborator Author

mnot commented Nov 15, 2019

I wouldn't spend too much time on the header name right now, if your intent is to take it to the WG.

@ioggstream
Copy link
Owner

Since last year, we have now various implementers (see #1); changing field names will be impractical now.

@mnot
Copy link
Collaborator Author

mnot commented Nov 19, 2020

That argument may not carry much weight once you get it into a WG. I'd suggest getting it there soon if you have implementations...

@ioggstream
Copy link
Owner

@mnot tomorrow we'll present the draft to the new httpapi-wg: we hope to call for adoption :) Reopening as requested.

@ioggstream ioggstream reopened this Nov 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants