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

Deduplicate header Separator logic #100179

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Conversation

MihaZupan
Copy link
Member

We have logic like this duplicated in 7 places:

string? separator = null;
if (headerValues.Length > 1)
{
    HttpHeaderParser? parser = header.Key.Parser;
    if (parser != null && parser.SupportsMultipleValues)
    {
        separator = parser.Separator;
    }
    else
    {
        separator = HttpHeaderParser.DefaultSeparator;
    }
}

This PR adds helpers that hide this. I also avoided the overhead of checking SupportsMultipleValues and doing the transcoding on the hot path.

As far as I can tell neither we nor Kestrel are using the HPack/QPack methods I removed which don't accept a value encoding.

@MihaZupan MihaZupan added this to the 9.0.0 milestone Mar 23, 2024
@MihaZupan MihaZupan self-assigned this Mar 23, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@MihaZupan MihaZupan requested a review from a team March 25, 2024 10:19
Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

@MihaZupan MihaZupan merged commit b42c516 into dotnet:main Mar 26, 2024
87 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants