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

Strict mode: Untagged network socket #46

Closed
saschpe opened this issue Aug 19, 2019 · 6 comments
Closed

Strict mode: Untagged network socket #46

saschpe opened this issue Aug 19, 2019 · 6 comments
Labels
bug Something isn't working

Comments

@saschpe
Copy link

saschpe commented Aug 19, 2019

Describe the bug
The underlying network socket managed by OkHttp isn't tagged according to policy. Strict mode complains about it. While it's a longstanding OkHttp issue other libraries apply a fixed tag.

To Reproduce
Enable strict mode in an app's Application class during onCreate():

if (BuildConfig.DEBUG) {
    StrictMode.enableDefaults()
}

Expected behavior
A clear and concise description of what you expected to happen.

Logs/Screenshots

2019-08-19 16:00:08.151 17277-17363/foo D/StrictMode: StrictMode policy violation: android.os.strictmode.UntaggedSocketViolation: Untagged socket detected; use TrafficStats.setThreadSocketTag() to track all network usage
        at android.os.StrictMode.onUntaggedSocket(StrictMode.java:2023)
        at com.android.server.NetworkManagementSocketTagger.tag(NetworkManagementSocketTagger.java:82)
...
        at okhttp3.internal.connection.RealConnection.connectSocket(RealConnection.java:245)
...

Library version
0.6.1

@saschpe saschpe added the bug Something isn't working label Aug 19, 2019
@colinrtwhite
Copy link
Member

colinrtwhite commented Aug 19, 2019

Thanks for the report. Do you have an example of other libraries working around this issue? I'm a little hesitant to work around this issue in the base Coil package. You can set a custom OkHttpClient which applies the work around like so:

Coil.setDefaultImageLoader {
    ImageLoader(this) {
         okHttpClient {
             // Apply the socket factory work-around.
         }
    }
}

@benjamin-bader
Copy link

If you decide not to add a tag in the base package, it might be nice to log a warning instead if the default, untagged, client is in use and strict-mode is on. If nothing else, it will save library users from googling the error and blaming Coil.

@colinrtwhite
Copy link
Member

@benjamin-bader Strict mode will already log a warning, which references OkHttp and not Coil.

@benjamin-bader
Copy link

This is true, but will lead people to think that coil is the problem. A helpful message would perhaps avert a number of spurious github issues, but if you think the existing message form strict mode is fine, then that's all that matters.

@colinrtwhite
Copy link
Member

Unfortunately, I don't think there's a way to show a message for this issue without requiring the user to install a custom socket factory. I'm going to close this issue for the moment + post the work-around as applied to Coil. If we get duplicate Github issues, we can point users back to this thread.

Here's the suggested work-around applied to Coil:

class TaggedSocketFactory(private val delegate: SocketFactory = getDefault()) : SocketFactory() {

    override fun createSocket(): Socket {
        val socket = delegate.createSocket()
        return configureSocket(socket)
    }

    override fun createSocket(host: String, port: Int): Socket {
        val socket = delegate.createSocket(host, port)
        return configureSocket(socket)
    }

    override fun createSocket(
        host: String,
        port: Int,
        localAddress: InetAddress,
        localPort: Int
    ): Socket {
        val socket = delegate.createSocket(host, port, localAddress, localPort)
        return configureSocket(socket)
    }

    override fun createSocket(host: InetAddress, port: Int): Socket {
        val socket = delegate.createSocket(host, port)
        return configureSocket(socket)
    }

    override fun createSocket(
        host: InetAddress,
        port: Int,
        localAddress: InetAddress,
        localPort: Int
    ): Socket {
        val socket = delegate.createSocket(host, port, localAddress, localPort)
        return configureSocket(socket)
    }

    private fun configureSocket(socket: Socket): Socket {
        TrafficStats.tagSocket(socket)
        return socket
    }
}

val imageLoader = ImageLoader(context) {
    val client = OkHttpClient.Builder().socketFactory(TaggedSocketFactory()).build()
    okHttpClient(client)
}

// If you use the Coil singleton...
Coil.setDefaultImageLoader(imageLoader)

@Kolyall
Copy link

Kolyall commented Apr 20, 2022

See https://stackoverflow.com/a/36840978/2425851
By this answer we need also before TrafficStats.tagSocket(socket) call TrafficStats.setThreadStatsTag . And call untagSocket after transfer of data

TrafficStats.setThreadStatsTag(0xF00D);
TrafficStats.tagSocket(outputSocket);
// Transfer data using socket
TrafficStats.untagSocket(outputSocket);

How to make it with Coil?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants