grpc-ecosystem / grpc-spring

Spring Boot starter module for gRPC framework.
https://grpc-ecosystem.github.io/grpc-spring/
Apache License 2.0
3.48k stars 812 forks source link

Maybe resolve a supplier in the DynamicSecurityHeaderCallCredentials in an executor to avoid blocking the thread #958

Open XAMeLeOH opened 1 year ago

XAMeLeOH commented 1 year ago

CallCredentials documentation suggests using asynchronous way of setting headers in case there is a blocking function in the code (e.g. a network call).

I would change this

        @Override
        public void applyRequestMetadata(final RequestInfo requestInfo, final Executor appExecutor,
                final MetadataApplier applier) {
            applier.apply(this.extraHeadersSupplier.get());
        }

To something like in this example

        @Override
        public void applyRequestMetadata(final RequestInfo requestInfo, final Executor appExecutor,
                final MetadataApplier applier) {
            executor.execute(new Runnable() {
                @Override
                public void run() {
                    try {
                        applier.apply(this.extraHeadersSupplier.get());
                    } catch (Throwable t) {
                        applier.fail(Status.UNAUTHENTICATED.withCause(e));
                    }
                }
            });
        }
fengli79 commented 10 months ago

Yes, this is the right approach. Setting the header is trivial, but the cost of getting the credential header's content could be a longer process, such as generate a token from a remote security provider, which should be offloaded to an async task without blocking the data plane. Would you like to contribute a PR?

ST-DDT commented 9 months ago

I created a PR to fix this.