Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CFNetwork] Ensure we do not block on redirects. #5900

Conversation

Projects
None yet
4 participants
@mandel-macaque
Copy link
Contributor

mandel-macaque commented Apr 15, 2019

The changes in mono are changing threading. This means that the code is
getting deadlock in the ReadStreamData.

This happens because we were not removing the handlers from the first request events.
Initially, before we had to take care about the auth headers, this was needed to init
certain internal state for the HttpContent that was exposed to the
user. With the fix of the CVE, the first request, in case of a redirect,
is garbage and is ignored. The second request is the one returned to the
user and contains all the required info.

Removing the event handlers, ensure that HandleCloseEvent is not called and
therefore ReadStream is never called.

[CFNetwork] Ensure we do not block on redirects.
The changes in mono are changing threading. This means that the code is
getting deadlock in the ReadStreamData.

This happens because we were not removing the handlers from the first requests events.
Initially, before we had to take care about the auth headers, this was needed to init
certain internal state for the HttpContent that was sexposed to the
user. With the fix of the CVE, the first request, in case of a redirect,
is garbage and is ignored. The second request is the one returned to the
user and contains all the required info.

Removing the event handlers, ensure that HandleClose is not called and
therefore ReadStream is never called.

@mandel-macaque mandel-macaque requested a review from akoeplinger Apr 15, 2019

@monojenkins

This comment has been minimized.

Copy link
Collaborator

monojenkins commented Apr 15, 2019

Build success
Build succeeded
API Diff (from stable)
API Diff (from PR only) (no change)
Generator Diff (no change)
Test run succeeded

@akoeplinger

This comment has been minimized.

Copy link
Member

akoeplinger commented Apr 16, 2019

I ran the monotouch-tests on simulator on two macs, and on my iPad. All passed 👍

@mandel-macaque mandel-macaque merged commit c734cf2 into xamarin:mono-2019-02 Apr 16, 2019

2 checks passed

Build Build success. No test results found.
Details
license/cla All CLA requirements met.

@mandel-macaque mandel-macaque deleted the mandel-macaque:fix-cfnetwork-handler-timeout branch Apr 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.