Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upConfigure header parsing to allow non-compliant headers #1144
Comments
blowdart
added
the
enhancement
label
Oct 3, 2016
blowdart
added this to the 1.1.0 milestone
Oct 3, 2016
This comment has been minimized.
This comment has been minimized.
|
In practice I expect this to become an FAQ where most people turn on UTF-8, so we should consider making that the default. Have we tested how these headers forward through IIS? WebListener? |
This comment has been minimized.
This comment has been minimized.
|
I like how we say "We" Nope, please try it :p |
muratg
assigned
cesarbs
Oct 6, 2016
This comment has been minimized.
This comment has been minimized.
|
@halter73 please work with @cesarbs, based on timing this may be load balanced to you. cc @davidfowl |
cesarbs
added
2 - Working
1 - Ready
and removed
2 - Working
1 - Ready
labels
Oct 28, 2016
muratg
modified the milestones:
1.2.0,
1.1.0
Oct 31, 2016
muratg
added
1 - Ready
and removed
2 - Working
labels
Oct 31, 2016
This comment has been minimized.
This comment has been minimized.
|
A big change (essentially a rewrite in header processing) so moving to 1.2.0. @Tratcher could you file a corresponding bug on WebListener repo? |
This comment has been minimized.
This comment has been minimized.
|
Should also fix: #1125 |
muratg
referenced this issue
Oct 31, 2016
Closed
Connection processing ends abnormally if referer header contains unencoded characters #1125
muratg
modified the milestones:
1.2.0,
2.0.0
Jan 12, 2017
cesarbs
added
2 - Working
and removed
1 - Ready
labels
Jan 17, 2017
This comment has been minimized.
This comment has been minimized.
|
Investigating this. Will check how those headers forward through IIS, and also what other servers do. |
This comment has been minimized.
This comment has been minimized.
|
Here's what I found so far:
A relevant bit from the RFC:
I think the most relevant part here is:
So it's not forbidding chars above 0x7F.
The most correct behavior seems to be to accept characters in the 0x80 - 0xFF range (which we reject at present), but to let the app decide the encoding. Http.sys appears to deviate from this though, by decoding as UTF-8. |
This comment has been minimized.
This comment has been minimized.
|
For reference, which header did you test? You may get different results for a common header vs a custom header. E.g. Host and Location are often special cased. |
This comment has been minimized.
This comment has been minimized.
|
How does Apache leave it up to the app to decode it? Does it expose the raw header bytes? |
This comment has been minimized.
This comment has been minimized.
cesarbs
added
the
1 - Ready
label
Jan 27, 2017
Tratcher
referenced this issue
Aug 10, 2018
Closed
non-ASCII request header cause BadHttpRequestException #2803
Tratcher
modified the milestones:
2.2.0,
2.2.0-preview2
Aug 10, 2018
This comment has been minimized.
This comment has been minimized.
|
Update on the investigation and recommendations. I've tested the following with the
A few notes on the behaviours:
Based on the comparisons, here are the recommendations and proposals:
These 3 points are independent and I think it's reasonable to start with 1 and 2. We can look into 3 if there is enough demand. Other alternatives that have been proposed:
But I don't think any of these alternatives are as desirable. |
This comment has been minimized.
This comment has been minimized.
|
@JunTaoLuo Can you look at response headers as well? |
This comment has been minimized.
This comment has been minimized.
|
I was hoping we could address response headers separately. Although related, apps have more control over the response headers whereas they cannot control what request headers are sent by clients.
|
This was referenced Aug 23, 2018
This comment has been minimized.
This comment has been minimized.
effyteva
commented
Aug 25, 2018
•
|
We ran into this issue as well. |
This comment has been minimized.
This comment has been minimized.
|
Parsing of request headers with UTF-8 encoded values has been merged and will be available in 2.2.0-preview2. |
This comment has been minimized.
This comment has been minimized.
|
I've continued to do some additional investigation in how servers handle header names, path and query strings with non-ascii characters and this is what I found:
Header names with non-ascii characters are almost universally rejected, other than nginx. We should follow the same pattern and continue to reject these requests. There's less consensus in how to treat requests with non-ascii characters in path and query string but these should be URL encoded. I think we can continue to reject these requests too, unless we have a compelling reason to do otherwise. |
This comment has been minimized.
This comment has been minimized.
effyteva
commented
Sep 5, 2018
|
I think it's highly critical to address this issue., at least in term of "removing" non-ascii headers. |
This comment has been minimized.
This comment has been minimized.
|
@effyteva I have already made the changes to accept UTF-8 encoded characters in header values, which means UTF-8 encoded urls in the |
This comment has been minimized.
This comment has been minimized.
effyteva
commented
Sep 5, 2018
|
@JunTaoLuo sorry, I didn't understand you already committed those changes for the 2.2 release. |
This comment has been minimized.
This comment has been minimized.
|
We decided to not pursue accepting non-ASCII characters in header names, path and query string at this time. If there is compelling reason to enable these scenarios, please file another issue and we will re-prioritize. A follow up issue to address UTF-8 values in response header values has been filed at https://github.com/aspnet/KestrelHttpServer/issues/2884. |
JunTaoLuo
closed this
Sep 5, 2018
JunTaoLuo
added
3 - Done
and removed
2 - Working
labels
Sep 5, 2018
JunTaoLuo
referenced this issue
Sep 6, 2018
Closed
Edge, when referer url contains Chinese characters, MVC will respond code 400 #8396
This comment has been minimized.
This comment has been minimized.
sandeepchauhan
commented
Oct 11, 2018
•
|
Thanks for this fix. Is there an ETA for 2.2.0 release? @JunTaoLuo |
This comment has been minimized.
This comment has been minimized.
|
See the roadmap. |
Tratcher
referenced this issue
Oct 12, 2018
Closed
Malformed request: invalid headers cause endpoints to give status code 400 #3004
This comment has been minimized.
This comment has been minimized.
turgayozgur
commented
Nov 23, 2018
•
|
We can encode. It is okay but what if we are migrating an old application to aspnetcore that already have too many customers who has the cookie contains non-ascii characters? For now, there is a workaround if you are using nginx at the front of your aspnetcore application. To remove all of the non ascii characters from the request header:
|
This was referenced Nov 27, 2018
This comment has been minimized.
This comment has been minimized.
effyteva
commented
Dec 8, 2018
|
We still suffer from the issue, after upgrading to ASP.NET Core 2.2. |
This comment has been minimized.
This comment has been minimized.
|
Comments on closed issues are not tracked, please open a new issue with the details for your scenario. |
blowdart commentedOct 3, 2016
•
edited by muratg
Rather than rejecting requests, allow configuration to either ignore UTF8 headers, or, parse them, even if they're illegal.
I suggest an enum Reject, Ignore, Parse.
However on output we should still be strict, UrlEncoding cookie values etc.
See #1076 and #1125
edit by @muratg: We should also consider request line when we get to this. #2647