Skip to content

Move IServerVariables feature and friends to Http.Abstractions #10167

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

Closed
analogrelay opened this issue May 10, 2019 · 6 comments
Closed

Move IServerVariables feature and friends to Http.Abstractions #10167

analogrelay opened this issue May 10, 2019 · 6 comments
Assignees
Labels
accepted This issue has completed "acceptance" testing (including accessibility) area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions help wanted Up for grabs. We would accept a PR to help resolve this issue

Comments

@analogrelay
Copy link
Contributor

analogrelay commented May 10, 2019

The IIS server adds an HTTP feature IServerVariablesFeature. Server Variables are not an IIS-exclusive concept and date all the way back to CGI so it seems reasonable that a server may want to provide them. We should move this type out of Servers.IIS and in to HttpAbstractions.

Specific changes:

@davidfowl
Copy link
Member

Maybe type forwarding is pointless since it won’t be compatible anyways

@Tratcher
Copy link
Member

Type forwarding IServerVariablesFeature would be compatible, no? Unless you change the namespace.

GetIISServerVariables can stay for compat, GetServerVariables would be a new extension method. Note GetServerVariables does not go in Http.Features, it would go into Http.Extensions.

@davidfowl
Copy link
Member

Ah it’s an extension method, I didn’t look at the interface

@mderriey
Copy link
Contributor

Cool, thanks for the clear instructions. I'll wait until #10022 is merged to take a stab at it.

@analogrelay
Copy link
Contributor Author

Updated instructions

@analogrelay analogrelay added this to the 3.0.0-preview7 milestone May 14, 2019
@analogrelay analogrelay added the help wanted Up for grabs. We would accept a PR to help resolve this issue label May 14, 2019
@jkotalik
Copy link
Contributor

jkotalik commented Jul 1, 2019

Acceptance checklist (check one item)

  • We decided not to take this fix.
  • The fix is tests-only.
  • The fix contains product changes (check all items below).
    • Relevant XML documentation comments for new public APIs are present.
    • Narrative docs (docs.microsoft.com) are updated. (check one item below)
      • The change requires a new article. An issue with an outline has been filed here: [ISSUE LINK]
      • The change requires a change to an existing article. A docs PR with these changes is linked here: [PR LINK]
      • The change requires no docs changes.
    • Verification has been completed. (check one item below)
      • Verified against ASP.NET Core App Version 3.0.0-preview7.19351.2

@jkotalik jkotalik added the accepted This issue has completed "acceptance" testing (including accessibility) label Jul 1, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted This issue has completed "acceptance" testing (including accessibility) area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
Development

No branches or pull requests

6 participants