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

Introduce ConnectionInterceptor for individual connection processing #129

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vtuan10
Copy link

@vtuan10 vtuan10 commented Mar 1, 2021

Motivation

Currently, it is only possible to set static http header values once. It is not possible to add header values which change with each network request.
We have the need to add headers which change with each request, because we sign each request (request method, request body and some other meta information) with a timestamp in our app.

What changed

Similarly to Okhttp, this introduces an interceptor interface ConnectionInterceptor to process each connection, i.e. add custom header which change with each request. It can be set at the beginning and will be called for each network request.

Example

ConnectionInterceptor interceptor = new ConnectionInterceptor() {
    @Override public HttpURLConnection intercept(HttpURLConnection connection, byte[] body) {
        connection.setRequestProperty("Prop", new Date().toString() + " super secret signature");
        return connection;
    }
};
Countly.sharedInstance().setConnectionInterceptor(interceptor)

Currently, it is only possible to set static http header values once. It
is not possible to add header values which change with each network
request. This introduces an interceptor interface to process each
connection, i.e. add custom header which change with each request.
@ArtursKadikis
Copy link
Member

ArtursKadikis commented Mar 1, 2021

Hello. This seems like something that would be useful.

Though the current implementation ignores the ImmediateRequestMaker which performs out of queue requests, and if set before init, it should be done through the config class.

If you would need to only set additional header values then why does the interceptor have the option to override the used HttpURLConnection?

Have you tried sending a user profile picture with this PR? Does it not interfare with sending that multipart request?

Is this special header information required for your firewall? Or what is exactly looking at that special header value?

@vtuan10
Copy link
Author

vtuan10 commented Mar 1, 2021

Thanks for your comment!

Though the current implementation ignores the ImmediateRequestMaker which performs out of queue requests, and if set before init, it should be done through the config class.

I moved the setting of the ConnectionInterceptor to the config class.
As far as I understood, ImmediateRequestMaker uses a ConnectionProcessor which is spawned by ConnectionQueue. This again, is created by Countly, thus, it always respects the CountlyConfig if available.

If you would need to only set additional header values then why does the interceptor have the option to override the used HttpURLConnection?

We implemented it in an interceptor style, because we did not want to intrude to much on the Countly codebase.
Also, we did not want to make specific assumptions for this functionality. In our specific case, we only use headers, but there are many other possibilities which an interceptor pattern enables such as conditional handling or even rewrites of the connection. And personally, I think, this is easier to understand (you get an connection and return a connection).

Have you tried sending a user profile picture with this PR? Does it not interfare with sending that multipart request?

We did not test this, because we do not use this feature. I will take a look again.

Is this special header information required for your firewall? Or what is exactly looking at that special header value?

We use Countly in a sensitive application which uses certain security measures. In this particular case, we implemented something called Device Binding. Every request our app creates has to be signed to prove that it was initialized by the registered device of the user.
Our backend verifies this on a logical level, requests are forwarded to a microservice and it is verified there. It then decides, whether the request is forwarded to Countly or is dropped.

@vtuan10 vtuan10 marked this pull request as draft March 1, 2021 23:41
@vtuan10
Copy link
Author

vtuan10 commented Mar 3, 2021

@ArtursKadikis
I changed the code for picturePath in a way, so that the 2 streams are generated, one for the actual output and one for the interceptor. The one for the output is using the old implementation, thus It is transparent to the interceptor.

Unfortunately, I was not able to test this, because even with setting picturePath as a userData property, I was unable to get any picturePath.
Can you give some support, in trying out the PR?

@vtuan10 vtuan10 marked this pull request as ready for review March 4, 2021 23:37
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.

2 participants