Draft: urlmon: test http with auto redirect disabled, and CreateBinding
Merge request reports
Activity
requested review from @jacek
assigned to @yshui
This test is meant to probe how native urlmon handles http redirect with auto redirect disabled, when the protocol object is created via
CreateBinding
. To my surprise, in this case native urlmon doesn't even callIBindingCallbackRedirect::Redirect
, it just does not handle http redirection.OTOH this test crashes wine, see Bug 55527
First of all, your test fails on Windows, so it's not interesting in current form. I will add more comments in !3725.
it fails on Windows because I didn't bother to remove that particular
CHECK_CALLED
. the reason it fails is because Windows' http protocol does not handle redirect.And we are trying to figure out how the native urlmon is different, in that case isn't this kind of test failures helpful? And this MR is a draft, I wasn't expect it to be merged in this form anyway.
I'll reopen this.
No, I wasn't upset and I'm sorry if I gave you that impression. I just don't see what could I possibly constructively review in this patch.
The result that you can see with your test is far from obvious in interpretation. All those tests are fairly complex. The difference may come from some other flags, the way we expose interfaces, the way we handle
Switch
/Continue
in tests, etc. I'm not convinced that the difference you're seeing is the root of the problem and even less convinced that it shouldn't generally be done inBindProtocol
.We have a test in 86f7c4c8 showing that
BindProtocol
object does handleBINDINFO_OPTIONS_DISABLEAUTOREDIRECTS
, we should probably extend it with emulated version based on result of http protocol behavior. You do that in !3725, but the remaining question is how do we distinguish http response codes.We have a test showing how
BINDINFO_OPTIONS_DISABLEAUTOREDIRECTS
is implemented in http protocol handler in 09b78c21, we should probably extend it with a test that posts data and check the behavior with different response statuses. Maybe it reports result somehow differently forHTTP_STATUS_REDIRECT_KEEP_VERB
? We don't know until we try and I was hoping you could check that. Such test may or may not be interesting, it depends on results we find. It's then all about how interesting Windows behavior is, nothing personal ;-)I just don't see what could I possibly constructively review in this patch.
Not this patch per se. I wanted to use this MR as the place for investigating how urlmon works in this particular case. Maybe this is not the place for that discussion, let me know if this should happen somewhere else.
This comment is very helpful btw, thanks.
All those tests are fairly complex.
Right. Ideally there could be a minimal test case representing how a typical user would use urlmon via
CreateBinding
, and we can check if and how native handles redirect in the simplest cases.We have a test in 86f7c4c8 showing that
BindProtocol
object does handleBINDINFO_OPTIONS_DISABLEAUTOREDIRECTS
AFAIK that's only when used with our ProtocolEmul. Maybe this is caused by differences in interfaces?
we should probably extend it with a test that posts data and check the behavior with different response statuses.
Yep, I can do that.
I was hoping you could check that.
Well, I can't read your mind. :-p
I was hoping you could check that.
Well, I can't read your mind. :-p
For the record, I did mention it in !3725 (comment 44407), clarified that over IRC and then again clarified it in !3725 (comment 45924).
You said we should add a test for POST request with real protocol, and I kind of did that. (It's true I didn't add a one for POST, but the reason why I stopped at GET is because with the real protocol it already behaved differently.)
It wasn't clear to me what you wanted me to get out of the test, which I only find out now:
Maybe it reports result somehow differently for
HTTP_STATUS_REDIRECT_KEEP_VERB
?Anyway, it's all clear now. I think we are getting better at understanding each other.
From my perspective, I thought I was asked to create a test that shows native urlmon does things differently, which I did. I was expecting someone with more knowledge on urlmon than me (e.g. you) to give me insight on how to continue the investigation. But then I was confused because I was told this test wasn't useful.
But now I know what I should look at next.
removed review request for @jacek