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.

Sign up for GitHub

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

Inconsistent behaviour when throwing StatusRuntimeException. #1158

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

Inconsistent behaviour when throwing StatusRuntimeException. #1158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 68 additions & 4 deletions tests/src/test/java/net/devh/boot/grpc/test/security/AbstractSecurityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,11 @@ DynamicTestCollection unprotectedCallTests() {
.add("unprotected-default",
() -> assertNormalCallSuccess(this.serviceStub, this.blockingStub, this.futureStub))
.add("unprotected-noPerm",
() -> assertNormalCallSuccess(this.noPermStub, this.noPermBlockingStub, this.noPermFutureStub));
() -> assertNormalCallSuccess(this.noPermStub, this.noPermBlockingStub, this.noPermFutureStub))
.add("unprotected-runtimeException-default",
() -> assertStatusRuntimeExceptionCallFailure(this.serviceStub, this.blockingStub,
this.futureStub,
PERMISSION_DENIED));
}

protected void assertNormalCallSuccess(final TestServiceStub serviceStub,
Expand All @@ -96,6 +100,16 @@ protected void assertNormalCallFailure(final TestServiceStub serviceStub,
TestServiceFutureStub::normal, expectedCode);
}

protected void assertStatusRuntimeExceptionCallFailure(final TestServiceStub serviceStub,
final TestServiceBlockingStub blockingStub,
final TestServiceFutureStub futureStub,
final Code expectedCode) {
assertUnaryFailingMethod(serviceStub,
TestServiceStub::statusRuntimeException, blockingStub,
TestServiceBlockingStub::statusRuntimeException, futureStub,
TestServiceFutureStub::statusRuntimeException, expectedCode);
}

/**
* Tests with unary call.
*
Expand All @@ -109,6 +123,9 @@ DynamicTestCollection unaryCallTest() {
() -> assertUnaryCallSuccess(this.serviceStub, this.blockingStub, this.futureStub))
.add("unary-noPerm",
() -> assertUnaryCallFailure(this.noPermStub, this.noPermBlockingStub, this.noPermFutureStub,
PERMISSION_DENIED))
.add("unary-runtimeException-default",
() -> assertUnaryStatusRuntimeExceptionCallFailure(this.serviceStub, this.blockingStub, this.futureStub,
PERMISSION_DENIED));
}

Expand All @@ -131,6 +148,17 @@ protected void assertUnaryCallFailure(final TestServiceStub serviceStub,
TestServiceFutureStub::secure, expectedCode);
}

protected void assertUnaryStatusRuntimeExceptionCallFailure(final TestServiceStub serviceStub,
final TestServiceBlockingStub blockingStub,
final TestServiceFutureStub futureStub,
final Code expectedCode) {
assertUnaryFailingMethod(serviceStub,
TestServiceStub::statusRuntimeException, blockingStub,
TestServiceBlockingStub::statusRuntimeException, futureStub,
TestServiceFutureStub::statusRuntimeException, expectedCode);
}


/**
* Tests with client streaming call.
*
Expand All @@ -142,7 +170,12 @@ DynamicTestCollection clientStreamingCallTests() {
return DynamicTestCollection.create()
.add("clientStreaming-default", () -> assertClientStreamingCallSuccess(this.serviceStub))
.add("clientStreaming-noPerm",
() -> assertClientStreamingCallFailure(this.noPermStub, PERMISSION_DENIED));
() -> assertClientStreamingCallFailure(this.noPermStub, PERMISSION_DENIED))
.add("clientStreaming-runtimeException-default",
() -> assertClientStreamingStatusRuntimeExceptionCallFailure(this.serviceStub,
PERMISSION_DENIED))

;
}

protected void assertClientStreamingCallSuccess(final TestServiceStub serviceStub) {
Expand All @@ -160,6 +193,15 @@ protected void assertClientStreamingCallFailure(final TestServiceStub serviceStu
assertFutureThrowsStatus(expectedCode, responseRecorder, 15, SECONDS);
}

protected void assertClientStreamingStatusRuntimeExceptionCallFailure(final TestServiceStub serviceStub,
final Code expectedCode) {
final StreamRecorder<Empty> responseRecorder = StreamRecorder.create();
final StreamObserver<SomeType> requestObserver = serviceStub.statusRuntimeExceptionDrain(responseRecorder);
// Let the server throw an exception if he receives that (assert security):
sendAndComplete(requestObserver, "explode");
assertFutureThrowsStatus(expectedCode, responseRecorder, 15, SECONDS);
}

/**
* Tests with server streaming call.
*
Expand All @@ -172,7 +214,11 @@ DynamicTestCollection serverStreamingCallTests() {
.add("serverStreaming-default",
() -> assertServerStreamingCallSuccess(this.serviceStub))
.add("serverStreaming-noPerm",
() -> assertServerStreamingCallFailure(this.noPermStub, PERMISSION_DENIED));
() -> assertServerStreamingCallFailure(this.noPermStub, PERMISSION_DENIED))
.add("serverStreaming-runtimeException-default",
() -> assertServerStreamingStatusRuntimeExceptionCallFailure(this.serviceStub,
PERMISSION_DENIED));

}

protected void assertServerStreamingCallSuccess(final TestServiceStub testStub) {
Expand All @@ -187,6 +233,13 @@ protected void assertServerStreamingCallFailure(final TestServiceStub serviceStu
assertFutureThrowsStatus(expectedCode, streamRecorder, 15, SECONDS);
}

protected void assertServerStreamingStatusRuntimeExceptionCallFailure(final TestServiceStub serviceStub,
final Code expectedCode) {
final StreamRecorder<SomeType> streamRecorder = StreamRecorder.create();
serviceStub.statusRuntimeExceptionSupply(EMPTY, streamRecorder);
assertFutureThrowsStatus(expectedCode, streamRecorder, 15, SECONDS);
}

/**
* Tests with bidirectional streaming call.
*
Expand All @@ -197,7 +250,9 @@ protected void assertServerStreamingCallFailure(final TestServiceStub serviceStu
DynamicTestCollection bidiStreamingCallTests() {
return DynamicTestCollection.create()
.add("bidiStreaming-default", () -> assertBidiCallSuccess(this.serviceStub))
.add("bidiStreaming-noPerm", () -> assertBidiCallFailure(this.noPermStub, PERMISSION_DENIED));
.add("bidiStreaming-noPerm", () -> assertBidiCallFailure(this.noPermStub, PERMISSION_DENIED))
.add("bidiStreaming-runtimeException-default",
() -> assertBidiStatusRuntimeExceptionCallFailure(this.serviceStub, PERMISSION_DENIED));
}

protected void assertBidiCallSuccess(final TestServiceStub testStub) {
Expand All @@ -214,6 +269,15 @@ protected void assertBidiCallFailure(final TestServiceStub serviceStub, final Co
assertFutureThrowsStatus(expectedCode, responseRecorder, 15, SECONDS);
}

protected void assertBidiStatusRuntimeExceptionCallFailure(final TestServiceStub serviceStub,
final Code expectedCode) {
final StreamRecorder<SomeType> responseRecorder = StreamRecorder.create();
final StreamObserver<SomeType> requestObserver = serviceStub.statusRuntimeExceptionBidi(responseRecorder);
sendAndComplete(requestObserver, "explode");
assertFutureThrowsStatus(expectedCode, responseRecorder, 15, SECONDS);
}


// -------------------------------------

protected void assertUnarySuccessfulMethod(final TestServiceStub serviceStub,
Expand Down
21 changes: 21 additions & 0 deletions tests/src/test/java/net/devh/boot/grpc/test/server/TestServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

import io.grpc.Context;
import io.grpc.Status;
import io.grpc.StatusRuntimeException;
import io.grpc.stub.StreamObserver;
import lombok.extern.slf4j.Slf4j;
import net.devh.boot.grpc.server.security.interceptors.AuthenticatingServerInterceptor;
Expand All @@ -48,6 +49,26 @@ public TestServiceImpl() {
log.info("Created TestServiceImpl");
}

@Override
public void statusRuntimeException(Empty request, StreamObserver<SomeType> responseObserver) {
throw new StatusRuntimeException(Status.PERMISSION_DENIED);
}

@Override
public StreamObserver<SomeType> statusRuntimeExceptionBidi(StreamObserver<SomeType> responseObserver) {
throw new StatusRuntimeException(Status.PERMISSION_DENIED);
}

@Override
public StreamObserver<SomeType> statusRuntimeExceptionDrain(StreamObserver<Empty> responseObserver) {
throw new StatusRuntimeException(Status.PERMISSION_DENIED);
}

@Override
public void statusRuntimeExceptionSupply(Empty request, StreamObserver<SomeType> responseObserver) {
throw new StatusRuntimeException(Status.PERMISSION_DENIED);
}

@Override
public void normal(final Empty request, final StreamObserver<SomeType> responseObserver) {
log.debug("normal");
Expand Down
14 changes: 13 additions & 1 deletion tests/src/test/proto/TestService.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ service TestService {

// Returns version information
rpc normal(google.protobuf.Empty) returns (SomeType) {}

// Fails with an error
rpc error(google.protobuf.Empty) returns (google.protobuf.Empty) {}

Expand All @@ -32,6 +32,18 @@ service TestService {
// Secured method with both multiple requests and answers
rpc secureBidi(stream SomeType) returns (stream SomeType) {}

// method
rpc statusRuntimeException(google.protobuf.Empty) returns (SomeType) {}

// method with only multiple requests
rpc statusRuntimeExceptionDrain(stream SomeType) returns (google.protobuf.Empty) {}

// method with only multiple answers
rpc statusRuntimeExceptionSupply(google.protobuf.Empty) returns (stream SomeType) {}

// method with both multiple requests and answers
rpc statusRuntimeExceptionBidi(stream SomeType) returns (stream SomeType) {}

}

message SomeType {
Expand Down