Skip to content

adjust ats settings in plist in pod install #38086

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

Closed
wants to merge 1 commit into from

Conversation

philIip
Copy link
Contributor

@philIip philIip commented Jun 27, 2023

Summary:
Changelog: [Internal]

in this diff, we update the developer's Info.plists to have the correct ATS settings

introducing usage of products: https://www.rubydoc.info/github/CocoaPods/Xcodeproj/Xcodeproj/Project#products-instance_method, in order to retrieve the plists

Differential Revision: D47041590

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Jun 27, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47041590

@analysis-bot
Copy link

analysis-bot commented Jun 27, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 9,045,368 +0
android hermes armeabi-v7a 8,294,642 +2
android hermes x86 9,561,545 -3
android hermes x86_64 9,403,893 -2
android jsc arm64-v8a 9,604,558 -1
android jsc armeabi-v7a 8,731,269 -1
android jsc x86 9,691,602 -1
android jsc x86_64 9,937,942 -4

Base commit: 605db44
Branch: main

philIip added a commit to philIip/react-native that referenced this pull request Jun 27, 2023
Summary:
Pull Request resolved: facebook#38086

Changelog: [Internal]

in this diff, we update the developer's Info.plists to have the correct ATS settings

introducing usage of products: https://www.rubydoc.info/github/CocoaPods/Xcodeproj/Xcodeproj/Project#products-instance_method, in order to retrieve the plists

Differential Revision: D47041590

fbshipit-source-id: 4118127e7be5374ff8bb7225ebdf6b00e06f7177
@philIip philIip force-pushed the export-D47041590 branch from 9481ff5 to fff1d60 Compare June 27, 2023 20:37
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47041590

philIip added a commit to philIip/react-native that referenced this pull request Jun 27, 2023
Summary:
Pull Request resolved: facebook#38086

Changelog: [Internal]

in this diff, we update the developer's Info.plists to have the correct ATS settings

introducing usage of products: https://www.rubydoc.info/github/CocoaPods/Xcodeproj/Xcodeproj/Project#products-instance_method, in order to retrieve the plists

Differential Revision: D47041590

fbshipit-source-id: b1ffc36478f37ece6006fd5360b058d94d076f41
@philIip philIip force-pushed the export-D47041590 branch from fff1d60 to 3a4ace8 Compare June 27, 2023 20:45
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47041590

philIip added a commit to philIip/react-native that referenced this pull request Jun 27, 2023
Summary:
Pull Request resolved: facebook#38086

Changelog: [Internal]

in this diff, we update the developer's Info.plists to have the correct ATS settings

introducing usage of files: https://www.rubydoc.info/github/CocoaPods/Xcodeproj/Xcodeproj/Project#files-instance_method, in order to retrieve the plists

Differential Revision: D47041590

fbshipit-source-id: 03a2d8759c50f58a2364c96f30900a44e47b3328
@philIip philIip force-pushed the export-D47041590 branch from 3a4ace8 to 0452f55 Compare June 27, 2023 22:21
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47041590

philIip added a commit to philIip/react-native that referenced this pull request Jun 27, 2023
Summary:
Pull Request resolved: facebook#38086

Changelog: [Internal]

in this diff, we update the developer's Info.plists to have the correct ATS settings

introducing usage of files: https://www.rubydoc.info/github/CocoaPods/Xcodeproj/Xcodeproj/Project#files-instance_method, in order to retrieve the plists

Differential Revision: D47041590

fbshipit-source-id: d2cf908a7b1c34f7f2e3523914a3f5facc503f2d
@philIip philIip force-pushed the export-D47041590 branch from 0452f55 to 1dc8797 Compare June 27, 2023 22:27
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47041590

Summary:
Pull Request resolved: facebook#38086

Changelog: [Internal]

in this diff, we update the developer's Info.plists to have the correct ATS settings

introducing usage of files: https://www.rubydoc.info/github/CocoaPods/Xcodeproj/Xcodeproj/Project#files-instance_method, in order to retrieve the plists

Reviewed By: cipolleschi

Differential Revision: D47041590

fbshipit-source-id: 14a1c182e0e7f0dbc105fc960a115bda0b3b8fd2
@philIip philIip force-pushed the export-D47041590 branch from 1dc8797 to a4858c2 Compare June 28, 2023 15:15
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47041590

@github-actions
Copy link

This pull request was successfully merged by @philIip in c86f15a.

When will my fix make it into a release? | Upcoming Releases

@github-actions github-actions bot added the Merged This PR has been merged. label Jun 28, 2023
@EvanBacon
Copy link
Contributor

Probably not safe to assume that all targets in the pbxproj are using React Native or connecting to a dev server. This change is modifying my widgets target to support loading. I also use widgets outside of my ios directory, which breaks here because the script is not tracing the Info.plist file relative to the parent groups.

info_plists = user_project
.files
.select { |p|
p.name&.end_with?('Info.plist')
Copy link
Contributor

Choose a reason for hiding this comment

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

Info.plist files should be discovered by tracing PBXProject > PBXNativeTarget > XCConfigurationList > XCBuildConfiguration > buildSettings.INFOPLIST_FILE.


def self.update_ats_in_plist(plistPaths, parent)
plistPaths.each do |plistPath|
fullPlistPath = File.join(parent, plistPath.path)
Copy link
Contributor

Choose a reason for hiding this comment

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

This would only be accurate if you trace the join the paths of PBXFileReference.path | PBXFileReference.name with every parent PBXGroup.path according to sourceTree. Iirc cocoapods has some utility method for getting the real file path for a file.

@@ -246,6 +246,7 @@ def react_native_post_install(
ReactNativePodsUtils.set_node_modules_user_settings(installer, react_native_path)
ReactNativePodsUtils.apply_flags_for_fabric(installer, fabric_enabled: fabric_enabled)
ReactNativePodsUtils.apply_xcode_15_patch(installer)
ReactNativePodsUtils.apply_ats_config(installer)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should ultimately only be running on the target that the user added the function to, and not all targets in the pbxproj.

gabrieldonadel added a commit to expo/expo that referenced this pull request Jan 23, 2024
# Why

On react-native 0.73 there was a change to the default template which
automatically sets `NSAllowsArbitraryLoads` to false after running `pod
install`. (facebook/react-native#38086)

Temporary fix for #26573

# How

As a temporary fix this PR updates the bare-minimum template to set a
custom ATS configuration for `exp.direct` in order to allow users to use
the `--tunnel` flag with expo cli.

Until a proper fix is not implemented upstream, If users need to
whitelist other HTTP domains they should manually add
`NSExceptionDomains` to their `infoPlist` via app.json

e.g.
```{
  "expo": {
    "ios": {
      "infoPlist": {
        "CFBundleAllowMixedLocalizations": true,
        "NSAppTransportSecurity": {
          "NSExceptionDomains": {
            "your-custom-domain.test": {
              "NSIncludesSubdomains": true,
              "NSExceptionAllowsInsecureHTTPLoads": true
            }
          }
        }
      }
    }
  }
}

```


# Test Plan

```
$ npx create-expo-app sdk50
$ cd sdk50  
$ npx expo prebuild --template /path/to/expo/expo/expo-template-bare-minimum-*.tgz
$ npx expo start --tunnel
```

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
gabrieldonadel added a commit to expo/expo that referenced this pull request Jan 23, 2024
On react-native 0.73 there was a change to the default template which
automatically sets `NSAllowsArbitraryLoads` to false after running `pod
install`. (facebook/react-native#38086)

Temporary fix for #26573

As a temporary fix this PR updates the bare-minimum template to set a
custom ATS configuration for `exp.direct` in order to allow users to use
the `--tunnel` flag with expo cli.

Until a proper fix is not implemented upstream, If users need to
whitelist other HTTP domains they should manually add
`NSExceptionDomains` to their `infoPlist` via app.json

e.g.
```{
  "expo": {
    "ios": {
      "infoPlist": {
        "CFBundleAllowMixedLocalizations": true,
        "NSAppTransportSecurity": {
          "NSExceptionDomains": {
            "your-custom-domain.test": {
              "NSIncludesSubdomains": true,
              "NSExceptionAllowsInsecureHTTPLoads": true
            }
          }
        }
      }
    }
  }
}

```

```
$ npx create-expo-app sdk50
$ cd sdk50
$ npx expo prebuild --template /path/to/expo/expo/expo-template-bare-minimum-*.tgz
$ npx expo start --tunnel
```

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants