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

UTF-8 BOM is added to raw value response when running on ASP .NET Core 3+ #628

Closed
VirusQuartirus opened this issue Jun 26, 2022 · 6 comments · Fixed by #630
Closed

UTF-8 BOM is added to raw value response when running on ASP .NET Core 3+ #628

VirusQuartirus opened this issue Jun 26, 2022 · 6 comments · Fixed by #630
Assignees
Labels
bug Something isn't working

Comments

@VirusQuartirus
Copy link
Contributor

VirusQuartirus commented Jun 26, 2022

Assemblies affected
ASP.NET Core OData 7.x

Describe the bug
When client sends request that expects raw value to be returned ($count for example) and doesn't specify Accept-Charset header UTF-8 BOM is written prior to content and Content-Type: text/plain. This happens when application is built using ASP .NET Core 5+ 3+.

Reproduce steps
See attached Fiddler logs.

Possible solution
It looks like ASP .NET Core 5 changed behavior of Microsoft.AspNetCore.Http.Headers.RequestHeaders class and now properties like AcceptEncoding AcceptCharset return empty collection instead of null value. It looks like it may be fixed in ODataOutputFormatter.WriteResponseHeaders change current line

if (requestHeader != null && requestHeader.AcceptCharset != null)

to

if (requestHeader?.AcceptCharset?.Count > 0)

FiddlerLogs.zip

@VirusQuartirus VirusQuartirus added the bug Something isn't working label Jun 26, 2022
@julealgon
Copy link
Contributor

The change in behavior you describe seems to indeed exist:

However, it appears to have been made in AspNetCore 3. You sure this only happens in 5+?

Also, you mention AcceptEncoding but then add code that mentions AcceptCharset. Was that intentional?

For future reference, actual link:

if (requestHeader != null && requestHeader.AcceptCharset != null)
{
IEnumerable<string> acceptCharsetValues = requestHeader.AcceptCharset.Select(cs => cs.Value.Value);
string newCharSet = string.Empty;
if (TryGetCharSet(currentContentType, acceptCharsetValues, out newCharSet))
{
currentContentType.Charset = new StringSegment(newCharSet);
response.Headers[HeaderNames.ContentType] = new StringValues(currentContentType.ToString());
}
}

@VirusQuartirus
Copy link
Contributor Author

VirusQuartirus commented Jun 28, 2022

However, it appears to have been made in AspNetCore 3. You sure this only happens in 5+?

Indeed, 3.x also suffers from this issue.

Also, you mention AcceptEncoding but then add code that mentions AcceptCharset. Was that intentional?

Just a typo. However all collection returning properties are affected.

@VirusQuartirus VirusQuartirus changed the title UTF-8 BOM is added to raw value response when running on ASP .NET Core 5+ UTF-8 BOM is added to raw value response when running on ASP .NET Core 3+ Jun 28, 2022
@KenitoInc
Copy link
Contributor

@VirusQuartirus
We welcome contributions. We will be happy to review your PR.

@VirusQuartirus
Copy link
Contributor Author

@VirusQuartirus We welcome contributions. We will be happy to review your PR.

I don't have write permissions to this repo.

@julealgon
Copy link
Contributor

I don't have write permissions to this repo.

That's usually how it works with public repositories. You don't create branches directly in the target repo: instead, you fork the repository to your account, make your branch and changes there, and then submit a pull request to the target repo from your fork.

This flow doesn't require you any kind of write permission on the target repo.

@VirusQuartirus
Copy link
Contributor Author

That's usually how it works with public repositories. You don't create branches directly in the target repo: instead, you fork the repository to your account, make your branch and changes there, and then submit a pull request to the target repo from your fork.

This flow doesn't require you any kind of write permission on the target repo.

Thanx for the tip. PR is created #630

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants