TechnikEmpire / CitadelCore

Cross platform filtering HTTP/S proxy based on .NET Standard 2.0.
Mozilla Public License 2.0
41 stars 15 forks source link

Body for blocked page not shown #60

Closed vdhong closed 6 years ago

vdhong commented 6 years ago

Hello, I use code below to show blocked page

messageInfo.ProxyNextAction = ProxyNextAction.DropConnection;
                messageInfo.BodyContentType = "text/html";
                messageInfo.Body=_blockBytes;

But browser show empty page, I use Inspect function of Chrome to see requesting details and it show response length is zero. I added log in to my code and CitadelCore code to see Body length and it's length is 25548 bytes: In FilterHttpResponseHandler.cs

if (requestMessageNfo.ProxyNextAction == ProxyNextAction.DropConnection)
                                        {
                                            LoggerProxy.Default.Info($"Drop with body {requestMessageNfo.Body.Length}");
                                            // ... the rest of code no change
                                            return;
                                        }

I try CitadelCore.Windows.Example project and it can show blocking content. I don't know why. Any idea?

TechnikEmpire commented 6 years ago

@vdhong If the connection is truly blocked but the block page doesn't show, then the response code you should see in chrome should be 204 No-Content. Can you confirm this?

If it's 204 no-content (which it sounds like), then some condition is failing where the payload is not being sent.

If it's 200 with no content, then the message info object is not being applied correctly. That can either be a bug how you're creating/modifying it, or it can be a bug in this engine quite possibly. What happens always is that ApplyMessageInfo is called on the various extension methods whenever it is detected that the library user has modified the payload themselves. You can find those methods in the Extensions folder:

https://github.com/TechnikEmpire/CitadelCore/tree/master/CitadelCore/Extensions

Specifically, see here for an example of how the logic works:

https://github.com/TechnikEmpire/CitadelCore/blob/master/CitadelCore/Extensions/HttpResponseExtensions.cs#L78

Let me know exactly what response code you got. That will narrow things down.

TechnikEmpire commented 6 years ago

@vdhong What might also be happening is that you're serving a HTTP 1.0 client. Is that possible? (I don't think so because you said chrome).

I'm going to open a bug that might arise, might be your issue but probably is not.

TechnikEmpire commented 6 years ago

Related to #61

vdhong commented 6 years ago

Chrome show empty and status code is 200. I think I saw a mistake in your code: Look FilterHttpResponseHandler.cs at line 145, you set MessageType = MessageType.Request and then at line 158 you call extension function await context.Response.ApplyMessageInfo(requestMessageNfo, context.RequestAborted); Look at HttpResponseExtensions.cs file line 82 of function ApplyMessageInfo you check message type is response then function apply changes to response if (messageInfo.MessageType == MessageType.Response) But message type of messageInfo is Request so that block code not run.

vdhong commented 6 years ago

By the way, I removed line if (messageInfo.MessageType == MessageType.Response) and the app run ok, blocked page show normal.

TechnikEmpire commented 6 years ago

Thanks for additional details and your help. The version 3.x change was a massive overhaul that I did in a very short period of time so I expected some hiccups.

TechnikEmpire commented 6 years ago

@vdhong Ah I see, this makes sense now. See ticket #49. I plan to enable users to modify responses and requests inline without dropping/killing the transaction.

I think I need to more clearly define the message info object to the user. It gives the user a lot of power, but also a lot of responsibility (spiderman quote!). So, as a user you need to set the message info type to Response before exiting your inspection function. That's the solution to this problem.

TechnikEmpire commented 6 years ago

Closing in favour of #62

TechnikEmpire commented 6 years ago

@vdhong This issue should be fixed now. Note that when you're modifying the message info object, the burden of ensuring that everything is set correctly falls on the user. If you look at the updated example source code, I now have to set the MessageType property to Response to get the block page to show. This confusion was caused by a poor design decision by me, as this property was previously read-only to the user.

vdhong commented 6 years ago

Thank you, Is it really need to check message type in extension function?

TechnikEmpire commented 6 years ago

@vdhong Just set it accordingly for now and it will work. I'll have to consider your objection to the check against my design plans. I am open to dropping the check if it isn't necessary, but I'd like to at least enforce some level of strictness or validation of the user's action. Right now it's kind of useless because it just silently fails when if anything, it should throw.

TechnikEmpire commented 6 years ago

@vdhong My primary concern is whether or not the nuget packages work now, because that's a critical bug so please let me know if that issue is resolved for you.