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

Remove Pseudo-Headers From Public API #42496

Merged

Conversation

Daniel-Genkin-MS-2
Copy link
Contributor

Currently we place the pseudo-headers in the HeaderNames.cs file which exposes them as public API. This is redundant as the pseudo-headers are all stripped out in internal code prior to passing it up to publicly available layers. Thus, these headers cannot actually be used by end users. This PR marks them as obsolete and adds them to an internal-only file, PseudoHeaderNames.cs.

From: #42002 (comment)

Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let @halter73 review this next week.

src/Http/Headers/src/HeaderNames.cs Outdated Show resolved Hide resolved
src/Http/Headers/src/Properties/AssemblyInfo.cs Outdated Show resolved Hide resolved
Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@halter73 will want to review this 😁

src/Servers/Kestrel/samples/http2cat/PseudoHeaderNames.cs Outdated Show resolved Hide resolved
src/Servers/Kestrel/samples/http2cat/PseudoHeaderNames.cs Outdated Show resolved Hide resolved
Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This looks good.

For APIs that people actually used, we should start using diagnostic IDs for obsoletions. See https://github.com/dotnet/designs/blob/main/accepted/2020/better-obsoletion/better-obsoletion.md. I don't think it's necessary for this though because these header names have never been populated in header dictionaries.

src/Http/Headers/src/HeaderNames.cs Outdated Show resolved Hide resolved
@Daniel-Genkin-MS-2 Daniel-Genkin-MS-2 merged commit 0240b1f into dotnet:main Jul 11, 2022
@ghost ghost added this to the 7.0-preview7 milestone Jul 11, 2022
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants