Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TSVart review of proxy-status #1591

Closed
2 of 3 tasks
mnot opened this issue Aug 5, 2021 · 2 comments
Closed
2 of 3 tasks

TSVart review of proxy-status #1591

mnot opened this issue Aug 5, 2021 · 2 comments

Comments

@mnot
Copy link
Member

mnot commented Aug 5, 2021

  • Section 2

Depending on the deployment, this might be a product or service name (e.g.,
ExampleProxy or "Example CDN"), a hostname ("proxy-3.example.com"), an IP
address, or a generated string.

Is really an IP address a good identifier for intermediary? Or is the case that
there some that doesn't have a better identity than its IP? And should there be
additional security considerations about including IP addresses in the header?

Editors: Yes, some proxies still use IP addresses (e.g., inside corporate infrastructure). It's not ideal. I think this is already covered by the warning about the potential exposure of internal infrastructure.


  • Section 2.1.1

Proxy Error Types can also define any number of extra parameters for use with
that type. Their use, like all parameters, is optional. As a result, if an
extra parameter is used with a Proxy Error Type for which it is not defined, it
will be ignored.

It is not obvious how these extra parameters are to be encoded.

If we take the example of DNS Error, how would that look like in an example?

HTTP/1.1 502 Bad Gateway
Proxy-Status: SomeReverseProxy; error=dns_error; rcode="123 something";
info-code=3454

Can you please clarify this aspect?

Editors: It's a structured field, so this is deterministic. What would you like clarified?


  • Section 3

Shouldn't the extra parameters in Section 2.3 be registered in the HTTP
Proxy-Status Parameters registry? If not can you clarify how they are handled?

Editors: They're only meaningful for those specific error types. See the guidelines for experts in both registries for conflict management.

@gloinul
Copy link

gloinul commented Aug 26, 2021

I understand the answer on Section 2 and I guess this is how it is.

Regarding 2.1.1: Having seen your answer and looking again at RFC 8941 I think I understand my confusion. What I hadn't grasped is that by declaring a header as a SF-list you are putting in place a formal parsing structure and does not additionally constrain it by formal syntax. Instead one one more informally declare what the items are, and in this case by simply stating there can be parameters the parameters syntax applies. I guess it is okay, potentially a reference to Section 3.1.2 in RFC 8941 could make this clearer in that cited paragraph.

For example "Proxy Error Types can also define any number of extra parameters (per Section 3.1.2 of RFC8941) for use with that type. "

Regarding Section 3: So I assume you are referring to this part of Section 2.2 when you reference conflict handling:

  • Parameter names should not conflict with registered extra
    parameters in the Proxy Error Type Registry.

So in the future if additional Proxy Errors are registered the expert and community needs to go through this registry to check all these registrations for extra parameters to avoid such conflicts. Is the intention here that all the extra parameters will be explicitly listed in the registry table, or only in supporting documentation? If it is the first, I guess it is straight forward and I have no further concerns.

mnot added a commit that referenced this issue Aug 27, 2021
@mnot
Copy link
Member Author

mnot commented Aug 27, 2021

Re: 2.1.1 - Parameters are introduced further up, so if anywhere, they should be ref'd there. See cfc8b02.

Re: 3 - that's correct.

@mnot mnot closed this as completed Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants