Skip to content

Commit

Permalink
Correctly handling custom reason phrase of status (helidon-io#3464)
Browse files Browse the repository at this point in the history
* Correctly handling custom reason phrase of status in webserver and webclient.

Signed-off-by: Tomas Langer <[email protected]>

* Fix to use correct status phrases

Signed-off-by: Tomas Langer <[email protected]>

* Checkstyle fix.

Signed-off-by: Tomas Langer <[email protected]>
  • Loading branch information
tomas-langer authored Oct 7, 2021
1 parent b04f7d9 commit 1ece4c1
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 33 deletions.
15 changes: 12 additions & 3 deletions common/http/src/main/java/io/helidon/common/http/Http.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2020 Oracle and/or its affiliates.
* Copyright (c) 2018, 2021 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -56,7 +56,16 @@ private Http() {
* Copied from JAX-RS.
*/
public enum Status implements ResponseStatus {

/**
* 100 Continue,
* see <a href="https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.1.1">HTTP/1.1 documentations</a>.
*/
CONTINUE_100(100, "Continue"),
/**
* 101 Switching Protocols,
* see <a href="https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.1.2">HTTP/1.1 documentations</a>.
*/
SWITCHING_PROTOCOLS_101(101, "Switching Protocols"),
/**
* 200 OK, see <a href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.2.1">HTTP/1.1 documentation</a>.
*/
Expand Down Expand Up @@ -632,7 +641,7 @@ private boolean reasonPhraseEquals(ResponseStatus other) {
@Override
public String toString() {
return "ResponseStatus{code=" + code()
+ ", reason" + reasonPhrase()
+ ", reason=" + reasonPhrase()
+ "}";
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -271,28 +270,7 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
}

private Http.ResponseStatus helidonStatus(HttpResponseStatus nettyStatus) {
final int statusCode = nettyStatus.code();

Optional<Http.Status> status = Http.Status.find(statusCode);
if (status.isPresent()) {
return status.get();
}
return new Http.ResponseStatus() {
@Override
public int code() {
return statusCode;
}

@Override
public Family family() {
return Family.of(statusCode);
}

@Override
public String reasonPhrase() {
return nettyStatus.reasonPhrase();
}
};
return Http.ResponseStatus.create(nettyStatus.code(), nettyStatus.reasonPhrase());
}

private static final class HttpResponsePublisher extends BufferedEmittingPublisher<DataChunk> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020 Oracle and/or its affiliates.
* Copyright (c) 2020, 2021 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -25,6 +25,7 @@
import io.helidon.webclient.WebClient;
import io.helidon.webclient.WebClientRequestBuilder;
import io.helidon.webclient.WebClientResponse;

import org.junit.jupiter.api.Test;

import static io.helidon.common.http.Http.Header.ORIGIN;
Expand Down Expand Up @@ -131,7 +132,9 @@ void test2PreFlightForbiddenOrigin() throws ExecutionException, InterruptedExcep
.toCompletableFuture()
.get();

assertThat(res.status(), is(Http.Status.FORBIDDEN_403));
Http.ResponseStatus status = res.status();
assertThat(status.code(), is(Http.Status.FORBIDDEN_403.code()));
assertThat(status.reasonPhrase(), is("CORS origin is not in allowed list"));
}

@Test
Expand Down Expand Up @@ -173,7 +176,9 @@ void test2PreFlightForbiddenMethod() throws ExecutionException, InterruptedExcep
.toCompletableFuture()
.get();

assertThat(res.status(), is(Http.Status.FORBIDDEN_403));
Http.ResponseStatus status = res.status();
assertThat(status.code(), is(Http.Status.FORBIDDEN_403.code()));
assertThat(status.reasonPhrase(), is("CORS origin is denied"));
}

@Test
Expand All @@ -192,7 +197,9 @@ void test2PreFlightForbiddenHeader() throws ExecutionException, InterruptedExcep
.toCompletableFuture()
.get();

assertThat(res.status(), is(Http.Status.FORBIDDEN_403));
Http.ResponseStatus status = res.status();
assertThat(status.code(), is(Http.Status.FORBIDDEN_403.code()));
assertThat(status.reasonPhrase(), is("CORS headers not in allowed list"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020 Oracle and/or its affiliates.
* Copyright (c) 2020, 2021 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -51,7 +51,9 @@ public void startupClient() {
void test1PreFlightAllowedOriginOtherGreeting() throws ExecutionException, InterruptedException {
WebClientResponse res = runTest1PreFlightAllowedOrigin();

assertThat(res.status(), is(Http.Status.FORBIDDEN_403));
Http.ResponseStatus status = res.status();
assertThat(status.code(), is(Http.Status.FORBIDDEN_403.code()));
assertThat(status.reasonPhrase(), is("CORS origin is denied"));
}

@AfterAll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,15 @@ public void writeStatusAndHeaders(Http.ResponseStatus status, Map<String, List<S
throw new IllegalStateException("Status and headers were already sent");
}

response = new DefaultHttpResponse(HTTP_1_1, valueOf(status.code()));
HttpResponseStatus nettyStatus;
if (status instanceof Http.Status || status.reasonPhrase() == null) {
// default reason phrase
nettyStatus = valueOf(status.code());
} else {
// custom reason phrase
nettyStatus = valueOf(status.code(), status.reasonPhrase());
}
response = new DefaultHttpResponse(HTTP_1_1, nettyStatus);
for (Map.Entry<String, List<String>> headerEntry : headers.entrySet()) {
response.headers().add(headerEntry.getKey(), headerEntry.getValue());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright (c) 2021 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.helidon.webserver;

import java.util.concurrent.TimeUnit;

import io.helidon.common.LogConfig;
import io.helidon.common.http.Http;
import io.helidon.webclient.WebClient;
import io.helidon.webclient.WebClientResponse;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import static io.helidon.common.http.Http.Status.BAD_REQUEST_400;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;

class ReasonPhraseTest {
public static final String CUSTOM_ERROR = "Custom error";
private static final Http.ResponseStatus CUSTOM_BAD_REQUEST = Http.ResponseStatus.create(BAD_REQUEST_400.code(),
CUSTOM_ERROR);
private static WebServer server;
private static WebClient client;

@BeforeAll
static void createWebServer() {
LogConfig.configureRuntime();
server = WebServer.builder()
.routing(Routing.builder()
.get("/default", ReasonPhraseTest::defaultCode)
.get("/custom", ReasonPhraseTest::customCode))
.build()
.start()
.await(10, TimeUnit.SECONDS);

client = WebClient.builder()
.baseUri("http://localhost:" + server.port())
.build();
}

@AfterAll
static void stopWebServer() {
if (server != null) {
server.shutdown()
.await(10, TimeUnit.SECONDS);
}
}

@Test
void testDefaultReasonPhrase() {
WebClientResponse response = client.get()
.path("/default")
.request()
.await(10, TimeUnit.SECONDS);

Http.ResponseStatus status = response.status();
assertThat(status.code(), is(400));
assertThat(status.reasonPhrase(), is(BAD_REQUEST_400.reasonPhrase()));

response.close();
}

@Test
void testCustomReasonPhrase() {
WebClientResponse response = client.get()
.path("/custom")
.request()
.await(10, TimeUnit.SECONDS);

Http.ResponseStatus status = response.status();
assertThat(status.code(), is(400));
assertThat(status.reasonPhrase(), is(CUSTOM_ERROR));

response.close();
}

private static void defaultCode(ServerRequest req, ServerResponse res) {
res.status(BAD_REQUEST_400)
.send();
}

private static void customCode(ServerRequest req, ServerResponse res) {
res.status(CUSTOM_BAD_REQUEST)
.send();
}
}

0 comments on commit 1ece4c1

Please sign in to comment.