-
Notifications
You must be signed in to change notification settings - Fork 191
Added OnResponseCompleted to HttpResponse #196
Conversation
@@ -6,9 +6,6 @@ | |||
using System.IO; | |||
using System.Linq; | |||
using System.Security.Claims; | |||
using System.Text; | |||
using System.Threading.Tasks; | |||
using Microsoft.AspNet.Http; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed unnecessary usings.
@@ -155,6 +155,16 @@ void IHttpResponseFeature.OnSendingHeaders(Action<object> callback, object state | |||
register(callback, state); | |||
} | |||
|
|||
void IHttpResponseFeature.OnResponseCompleted(Action<object> callback, object state) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may also want to expose it in the OWIN environment like OnSendingHeaders
: https://github.com/aspnet/HttpAbstractions/blob/response-feature/src/Microsoft.AspNet.Owin/OwinEnvironment.cs#L64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This?
HttpAbstractions/src/Microsoft.AspNet.Owin/OwinFeatureCollection.cs
Lines 150 to 158 in 08ddbe8
void IHttpResponseFeature.OnSendingHeaders(Action<object> callback, object state) | |
{ | |
var register = Prop<Action<Action<object>, object>>(OwinConstants.CommonKeys.OnSendingHeaders); | |
if (register == null) | |
{ | |
throw new NotSupportedException(OwinConstants.CommonKeys.OnSendingHeaders); | |
} | |
register(callback, state); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, the code you pointed supports calling the server.OnSendingHeaders
callback from ASP.NET 5, but can't inject server.OnSendingHeaders
in the OWIN environment, which is what the OwinEnvironment
class does.
Anyway, @Tratcher said they were only using community defined keys.
Have you considered making this new hook async? Related discussion (concerning OSH): aspnet/Security#76 |
@@ -70,6 +70,7 @@ internal static class CommonKeys | |||
public const string AppName = "host.AppName"; | |||
public const string Capabilities = "server.Capabilities"; | |||
public const string OnSendingHeaders = "server.OnSendingHeaders"; | |||
public const string OnResponseCompleted = "server.OnResponseCompleted"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is a key http://owin.org/spec/spec/CommonKeys.html
|
@ajaybhargavb rebase this |
fef2113
to
0d8af4a
Compare
Rebased. |
|
Can you also look at all the related PRs? |
390f310
to
ee016e1
Compare
ee016e1
to
f637027
Compare
Issue #160 @lodejard @Tratcher @GrabYourPitchforks