Flurl: Cookies and DelegatingHandler

[sandorfr]

When wrapping the IHttpMessageHandler with a delegatingHandler, the current implementation tries to directly set the Cookie header instead of using the CookieContainer. The cookie header is then overriden by the underlying HttpClientHandler. So it drops the cookies if not present in the underlying container.

This creates a hard to debug sort of bug. I think a quick and easy way to fix that is to add support for Delegating Handler (maybe recursive in case there are several of them). My quick fix for my use case is that : https://github.com/tmenier/Flurl/compare/master...sandorfr:cookie-delegating-handler-support

I'm happy to take the time to write a better fix and submit a pull request if you're interested.

[tmenier]

Thanks for reporting, I can how that would happen. You're on the right track with your fix but I think you'd need to check DelegatingHandler.InnerHandler recursively, because who knows how many of them might be in the pipeline. It'll need a unit test as well. Feel free to take a crack at it, otherwise I can take it.

[sandorfr]

Sorry I didn't find the time to answer earlier. I submitted a PR with two tests and the recursive support. let met know what you think.

[tmenier]

Thanks, this looks like the right idea for sure. I'm deep into Flurl.Http 2.0 right now so it may not get merged any sooner than that. Are you able to work around it for now?

[sandorfr]

No urge on my side, I'm using a custom build from a private feed

[sandorfr]

@tmenier did you get a chance to have a look at it? Would be good to have it in the 2.0 release :)
Kind regards

[tmenier]

Yes, it'll be fixed in 2.0.

[sandorfr]

Did you notice the pull request i submitted ?

[tmenier]

Noticed it, yes. Reviewed it, not yet. Code changes for 2.0 will be substantial, so I'm not sure yet whether it will merge cleanly as-is, but one way or another it'll be fixed.

[sandorfr]

Cool. I can do a PR against dev branch if it is of any help. Let me know if it does.

[tmenier]

There were merge conflicts with 2.0 as I feared but it was easy enough to apply your changes manually. Your implementation and tests looked great. Other than minor formatting tweaks I kept your logic as-is. Thanks again.