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

added option to include generic/permissive network security config file durin… #2791

Merged
merged 11 commits into from
May 7, 2022

Conversation

erev0s
Copy link
Contributor

@erev0s erev0s commented Mar 31, 2022

Added the flag '-n' as an option to be able to add a generic permissive Network Security Configuration file during the build process. This is based on the following feature request #1622

fixes: #1622

Copy link
Owner

@iBotPeaches iBotPeaches left a comment

Choose a reason for hiding this comment

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

I do think its crucial for this new feature to produce 2 tests that:

  • modify an application that already has a network config
  • modify an application that does not have one

I understand if the Apktool codebase may not be the easiest to do that, but if you want to take a stab at that it would be appreciated.

Thanks for the PR.

@erev0s
Copy link
Contributor Author

erev0s commented Apr 1, 2022

indeed i was thinking that tests might be useful for it but hesitated to add them initially. I have made a new push with the tests. Used the testapp you already have in there and added the configuration to it.

@iBotPeaches
Copy link
Owner

I'm going to take the branch from here to slim down the test app to just the basics so we don't replicate the large test application then push up some changes.

@iBotPeaches
Copy link
Owner

Okay made some commits, but can't seem to push to this branch. If you want to pluck these

@erev0s
Copy link
Contributor Author

erev0s commented Apr 6, 2022

remote add and then cherry-pick: done

@erev0s
Copy link
Contributor Author

erev0s commented Apr 17, 2022

@iBotPeaches any updates here

@iBotPeaches
Copy link
Owner

@iBotPeaches any updates here

Sorry not yet. Appears I broke build w/ my latest patches since I was working on Mac and haven't visited Linux yet.

@iBotPeaches
Copy link
Owner

Two additional commits since I cannot push to this :/

@erev0s
Copy link
Contributor Author

erev0s commented Apr 25, 2022

@iBotPeaches added them

@iBotPeaches
Copy link
Owner

@erev0s - okay just just fix that last conflict, which was caused by my merge last night and then good to go.

@erev0s
Copy link
Contributor Author

erev0s commented Apr 30, 2022

@iBotPeaches did a push with what seems to be the conflict. It does not allow me to directly resolve the conflict through the web interface though - it now worked

@iBotPeaches iBotPeaches merged commit 8fab4bf into iBotPeaches:master May 7, 2022
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.

Feature request: Add an option to include a generic Network Security Configuration file to the output APK
2 participants