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

Better proguard rule #8399

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Better proguard rule #8399

wants to merge 1 commit into from

Conversation

vvb2060
Copy link
Contributor

@vvb2060 vvb2060 commented May 6, 2024

Better fix #7827, no need to keep names #8156

@mtotschnig
Copy link

Version 5.0.0-alpha.14 with R8 currently crashes with java.lang.IllegalStateException: Unable to load /okhttp3/internal/publicsuffix/a.gz resource from the classpath. This seems to fix it. Can this be taken up for the next alpha?

@yschimke
Copy link
Collaborator

yschimke commented Jul 6, 2024

Yep.

We actually have what I hoped were strong tests for this https://github.com/square/okhttp/blob/parent-5.0.0-alpha.14/android-test-app/src/androidTest/kotlin/okhttp/android/testapp/PublicSuffixDatabaseTest.kt

Unfortunately we are sort of pinned on AGP 8.2, so it wasn't testing this effectively.

Until I untangle that, I'll try to reproduce the issue with a hacked build, and then apply the fix.

@yschimke
Copy link
Collaborator

yschimke commented Jul 6, 2024

@mtotschnig I can't reproduce that error, even bumping to AGP 8.5. Are you able to provide a repro?

At the moment this seems like a nice to have, improving the names. But I'd like to understand why it's failing for you.

@mtotschnig
Copy link

@yschimke I tried with a minimal example, but was not able to reproduce yet.
This is the deobfuscated stacktrace:

java.lang.IllegalStateException: Unable to load /okhttp3/internal/publicsuffix/a.gz resource from the classpath.
at okhttp3.internal.publicsuffix.PublicSuffixDatabase.findMatchingRule(PublicSuffixDatabase.kt:114)
at okhttp3.internal.publicsuffix.PublicSuffixDatabase.getEffectiveTldPlusOne(PublicSuffixDatabase.kt:75)
at okhttp3.Cookie$Companion.parse$okhttp(Cookie.kt:561)
at okhttp3.Cookie$Companion.parse(Cookie.kt:450)
at okhttp3.Cookie$Companion.parseAll(Cookie.kt:710)
at okhttp3.internal.http.HttpHeaders.receiveHeaders(HttpHeaders.kt:211)
at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.kt:85)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:126)
at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:72)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:126)
at de.telekom.net.http.client.RestClient_extKt$getOkHttpClient$lambda$2$$inlined$-addInterceptor$1.intercept(OkHttpClient.kt:1408)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:126)
at okhttp3.internal.connection.RealCall.getResponseWithInterceptorChain$okhttp(RealCall.kt:203)
at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:527)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
at java.lang.Thread.run(Thread.java:920)

So you need a situation with Cookies, but I tried to directly put the offending call
PublicSuffixDatabase.get().getEffectiveTldPlusOne(domain)
into my app code, and it does not crash with minifyEnabled.
I cannot share details about the project where this happens, so will invest more time to extract the context that triggers this.

@mtotschnig
Copy link

I was trying to find differences between the project where the problem arises, and the minimal problem where it does not, and align them with respect to Kotlin and Java versions, but was not able to find out what makes the difference.
When I analyze the apk with apktool, in the failing project, I find
/unknown/md/a.gz
versus
/unknown/okhttp3/internal/publicsuffix/a.gz
in the minimal project.

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