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

[core] fix self-signed connections in network inspector #32670

Merged

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Nov 6, 2024

Why

fixes broken self-signed connection from network inspector because we intercepted the URLSessionDelegate. the default delegate disallows self-signed connection.
fixes #24096
close ENG-9914

How

  • add urlSession(_:task:didReceive:completionHandler:) handler
  • pass the completion handler to URLProtocolClient, which would be the original URLSessionDelegate handler
  • since the URLProtocolClient's urlProtocol(_:didReceive:) is an old interface and not having a completion handler. this pr tries to introduce an URLAuthenticationChallengeForwardSender to forward the completion handler to URLAuthenticationChallengeSender

Test Plan

Checklist

Copy link

linear bot commented Nov 6, 2024

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Nov 6, 2024
@expo-bot
Copy link
Collaborator

expo-bot commented Nov 6, 2024

The Pull Request introduced fingerprint changes against the base commit: 2f86ce5

Fingerprint diff
[
  {
    "op": "changed",
    "source": {
      "type": "dir",
      "filePath": "../../packages/expo-modules-core",
      "reasons": [
        "expoAutolinkingIos",
        "expoAutolinkingAndroid"
      ],
      "hash": "fb873df3916bb3a36d68cf9d41e9cbcf6eda5c7a"
    }
  }
]

Generated by PR labeler 🤖

@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Nov 6, 2024
@Kudo Kudo marked this pull request as ready for review November 6, 2024 19:40
@Kudo Kudo requested a review from tsapeta November 6, 2024 19:40
Copy link
Contributor

github-actions bot commented Nov 6, 2024

Subscribed to pull request

File Patterns Mentions
packages/expo-modules-core/** @tsapeta, @Kudo, @lukmccall

Generated by CodeMention

@brentvatne brentvatne merged commit 579f4c7 into main Nov 7, 2024
22 checks passed
@brentvatne brentvatne deleted the @kudo/eng-9914-sdk-49-self-signed-connection-broke-in-sdk-49 branch November 7, 2024 00:02
@brentvatne brentvatne added the published Changes from the PR have been published to npm label Nov 7, 2024
@Kudo Kudo mentioned this pull request Nov 14, 2024
3 tasks
Kudo added a commit that referenced this pull request Nov 14, 2024
# Why

fix expo/fetch stuck
reported from
#21710 (reply in thread)

# How

it's a regression from #32670. the delegate proxy also being used for
expo/fetch. the delegate does not implement the
`URLSessionTaskDelegate.urlSession(_:task:didReceive:completionHandler:)`
method. so completionHandler does not be called and get stuck.

this pr checks whether the delegate method is implemented, otherwise
uses the default impelmentatiion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[sdk 49] Self-signed connection broke in SDK 49
4 participants