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 bundled proguard rules #2092

Merged
merged 5 commits into from
Dec 2, 2022
Merged

Added bundled proguard rules #2092

merged 5 commits into from
Dec 2, 2022

Conversation

shanshin
Copy link
Contributor

@shanshin shanshin commented Nov 8, 2022

Resolves #1121
Resolves #1899
Resolves #1900
Resolves #2050

Resolves #1121
Resolves #1899
Resolves #1900
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
Co-authored-by: Leonid Startsev <sandwwraith@users.noreply.github.com>
Copy link
Member

@sandwwraith sandwwraith left a comment

Choose a reason for hiding this comment

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

Thank you for the thorough investigation!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
rules/r8.pro Outdated Show resolved Hide resolved
Co-authored-by: Leonid Startsev <sandwwraith@users.noreply.github.com>
@bubenheimer
Copy link

bubenheimer commented Jan 31, 2023

I think an additional README comment should be added regarding Proguard/R8 usage with obfuscation. In this case serialized class and field names will be obfuscated, and will be non-stable. Personally I use @SerialName annotations to preserve serialized name stability, but others might opt for additional keep rules. (Just don't make those default rules, please!)

@sandwwraith
Copy link
Member

Serialized class names are stored in the descriptor, so it shouldn't affect polymorphic serialization, only error messages would be altered

@bubenheimer
Copy link

bubenheimer commented Jan 31, 2023

The new README reads as if the new default Proguard rules took care of any minimization/obfuscation issues, but that's not true. Obfuscated names are not stable, so the next app build will not be able to read back some of the serialized data, at least with JSON serialization. Pretty sure I tested this.

By default, proguard rules are supplied with the library. These rules keep serializers for all serializable classes that are retained after shrinking, so you don't need additional setup.

Ideally the README could mention the problem with obfuscation and highlight various potential solutions, like @SerialName and custom user keep rules and annotations.

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.

None yet

4 participants