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

OkHttp extension ships with a ProGuard rule that breaks newer versions of OkHttp #10310

Closed
1 task
Stonos opened this issue Jun 1, 2022 · 6 comments
Closed
1 task
Assignees
Labels

Comments

@Stonos
Copy link

Stonos commented Jun 1, 2022

ExoPlayer Version

2.17.1

Devices that reproduce the issue

All

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

  1. Add the following to your build.gradle file:
implementation 'com.squareup.okhttp3:okhttp:5.0.0-alpha.7'
implementation 'com.google.android.exoplayer:extension-okhttp:2.17.1'
  1. Add the following code somewhere:
PublicSuffixDatabase.get().getEffectiveTldPlusOne("example.com")
  1. Compile a release version of the app with minifyEnabled true

Expected result

getEffectiveTldPlusOne should return example.com.

Actual result

getEffectiveTldPlusOne crashes with the following exception:

java.lang.IllegalStateException: Unable to load PublicSuffixDatabase.gz resource from the classpath.
        at okhttp3.internal.publicsuffix.PublicSuffixDatabase.a(SourceFile:501)

This happens because of this consumer rule that ships with the OkHttp extension:

-keepnames class okhttp3.internal.publicsuffix.PublicSuffixDatabase

It probably stopped working due to this change in OkHttp: square/okhttp#6974

Media

N/A

Bug Report

@marcbaechinger
Copy link
Contributor

Thanks for reporting.

I can repro this behaviour with the steps from above. I can also confirm that removing the mentioned line from the proguard rules solves the problem. It seems to work with 4.9.0 as well when I remove this line.

I'm not sure why we had this line actually. We followed the advice on the okhttp README at that time when it has been added.

Will check with okhttp folks.

@marcbaechinger
Copy link
Contributor

marcbaechinger commented Jun 1, 2022

This page suggests we can just remove our proguard config at least for gradle: https://square.github.io/okhttp/features/r8_proguard/

@Stonos
Copy link
Author

Stonos commented Jun 1, 2022

Thanks for taking a look!

I think that it should be safe to delete the proguard-rules.txt file from the extension, since nowadays OkHttp is bundled with its own rules which should get picked up by R8.

I'm not sure why we had this line actually. We followed the advice on the okhttp README at that time when it has been added.

I believe that this is because at the time OkHttp didn't have embedded rules. The extension's rules were added on 2018-04-03, and OkHttp's rules were added on 2018-06-15.

@yschimke
Copy link
Contributor

yschimke commented Jun 1, 2022

Yep - please remove any lines related to OkHttp from your file.

Basically since 3.11.0 our advice has been to rely on the embedded rules. So we stripped out all the other rules, and have taken advantage of that when moving files around.

@nuclearmagneton
Copy link

Any update on this?

@marcbaechinger
Copy link
Contributor

Sorry, I haven't reference the commit in this issue. This landed on dev-v2 with this change:
759de7d

I'm closing this issue. Please let me know if you think there is something missing. Thanks.

@google google locked and limited conversation to collaborators Nov 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants