Media preview crash on `www.example.com…`

[ncfavier]

With media previews enabled, on latest master, the app crashes when trying to fetch a media preview for the string www.example.com… (note that the last character is a Unicode U+2026 ellipsis, it probably happens with other characters but I have not tested). Doesn't happen with example.com….

The crash is especially annoying to recover from because any buffers adjacent to the one containing the string will crash as well (I guess the app pre-emptively renders adjacent buffers).

I'll provide logs later.

[ncfavier]

With insecure requests set to rewrite as HTTPS:

06-01 15:14:56.769  7144 21199 E AndroidRuntime: FATAL EXCEPTION: OkHttp Dispatcher
06-01 15:14:56.769  7144 21199 E AndroidRuntime: Process: com.ubergeek42.WeechatAndroid.dev, PID: 7144
06-01 15:14:56.769  7144 21199 E AndroidRuntime: java.lang.IllegalArgumentException: unexpected host: www.example.com...
06-01 15:14:56.769  7144 21199 E AndroidRuntime:    at okhttp3.Address.<init>(Address.kt:14)
06-01 15:14:56.769  7144 21199 E AndroidRuntime:    at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:30)
06-01 15:14:56.769  7144 21199 E AndroidRuntime:    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:14)
06-01 15:14:56.769  7144 21199 E AndroidRuntime:    at com.ubergeek42.WeechatAndroid.media.OkHttpSecuringInterceptor.intercept(OkHttpSecuringInterceptor.java:30)
06-01 15:14:56.769  7144 21199 E AndroidRuntime:    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:14)
06-01 15:14:56.769  7144 21199 E AndroidRuntime:    at okhttp3.internal.connection.RealCall.getResponseWithInterceptorChain$okhttp(RealCall.kt:25)
06-01 15:14:56.769  7144 21199 E AndroidRuntime:    at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:12)
06-01 15:14:56.769  7144 21199 E AndroidRuntime:    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
06-01 15:14:56.769  7144 21199 E AndroidRuntime:    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
06-01 15:14:56.769  7144 21199 E AndroidRuntime:    at java.lang.Thread.run(Thread.java:784)

Interestingly the … is now ... (and that's not an actual ellipsis when printing the error, I tried with a shorter host).

---

With insecure requests allowed:

06-01 15:31:02.233 22974 23070 E AndroidRuntime: FATAL EXCEPTION: OkHttp Dispatcher
06-01 15:31:02.233 22974 23070 E AndroidRuntime: Process: com.ubergeek42.WeechatAndroid.dev, PID: 22974
06-01 15:31:02.233 22974 23070 E AndroidRuntime: java.lang.IllegalArgumentException: unexpected host: www.ajeojc.com...
06-01 15:31:02.233 22974 23070 E AndroidRuntime:    at okhttp3.Address.<init>(Address.kt:14)
06-01 15:31:02.233 22974 23070 E AndroidRuntime:    at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:30)
06-01 15:31:02.233 22974 23070 E AndroidRuntime:    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:14)
06-01 15:31:02.233 22974 23070 E AndroidRuntime:    at okhttp3.internal.connection.RealCall.getResponseWithInterceptorChain$okhttp(RealCall.kt:25)
06-01 15:31:02.233 22974 23070 E AndroidRuntime:    at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:12)
06-01 15:31:02.233 22974 23070 E AndroidRuntime:    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
06-01 15:31:02.233 22974 23070 E AndroidRuntime:    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
06-01 15:31:02.233 22974 23070 E AndroidRuntime:    at java.lang.Thread.run(Thread.java:784)

With insecure requests disallowed, no crash.

[ncfavier]

I think there are two parts to the issue:

1. excluding characters like … from the URL regex, at least in the TLD part, and
2. making sure that exceptions like these are caught, but I'm not sure if that should be solved in weechat-android or in okhttp given the latter stack trace.

[oakkitten]

Thanks for the detailed report! I think this is a problem with … specifically as apparently it should be converted to ... when idna-encoded. See also this issue. I think it's kind of a bug in OkHttp, since this is true:

new Request.Builder().url("http://…/").url.toString().equals("http://.../")

but this raises IllegalArgumentException:

new Request.Builder().url("http://.../")

Essentially, OkHttp creates a request that it itself can't use.

[oakkitten]

This should fix it, what do you think?

[ncfavier]

Looks good! Why did you add a ? on line 122?

[oakkitten]

This makes tld non-greedy, which makes it find http://foo.bar instead of http://foo.bar” in “http://foo.bar”. Have to be a bit careful with stuff like this as this can affect performance, I hope this is ok.

[ncfavier]

Ah right, I thought it was making the TLD optional. LGTM then.

[ncfavier]

Anything blocking this?

[oakkitten]

Oh it's just that a few other things came up and we want to sort them out as well. If … is an immediate problem for you you can meanwhile use a branch build from here, one with urls-with-ellipses in the name

[ncfavier]

Ah, thanks