8000 [v8] Fix deprecation in GULNetworkURLSession.m by ncooke3 · Pull Request #193 · google/GoogleUtilities · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[v8] Fix deprecation in GULNetworkURLSession.m #193

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

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

ncooke3
Copy link
Member
@ncooke3 ncooke3 commented Jun 20, 2024

Part of effort to close out #176

Comment on lines -410 to -411
shouldAllow =
(trustEval == kSecTrustResultUnspecified || trustEval == kSecTrustResultProceed);
Copy link
Member Author
@ncooke3 ncooke3 Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Apple docs: The SecTrustEvaluateWithError(::) method returns a pass or fail indicator and an error describing the reason for any failure. So, I think it's reasonable to rely on the shouldAllow result.

Furthermore, GTMSessionFetcher relies on the API's return bool value:

https://github.com/google/gtm-session-fetcher/blob/4990f8956605007c2546824d57432f04dc37fe70/Sources/Core/GTMSessionFetcher.m#L2615-L2628

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM! Nice cleanup.

@google-oss-bot
Copy link

Coverage Report 1

Affected Products

8000
  • GoogleUtilities-ios-unit-GoogleUtilities.framework

    Overall coverage changed from 0.62% (cb3d41f) to 0.14% (754cedf) by -0.49%.

    FilenameBase (cb3d41f)Merge (754cedf)Diff
    GULAppEnvironmentUtil.m3.23%3.42%+0.20%
    GULNetworkInfo.m23.86%0.00%-23.86%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/PbfRNhZkOf.html

@ncooke3 ncooke3 requested a review from andrewheard June 21, 2024 18:51
Comment on lines -410 to -411
shouldAllow =
(trustEval == kSecTrustResultUnspecified || trustEval == kSecTrustResultProceed);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM! Nice cleanup.

@ncooke3 ncooke3 merged commit 7e0a762 into release-8.0 Jun 21, 2024
19 checks passed
@ncooke3 ncooke3 deleted the nc/fix-deprecation branch June 21, 2024 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0