Skip to content

Commit

Permalink
Review comments; remove unneeded newly-introduced isSecure method on …
Browse files Browse the repository at this point in the history 
…CORS request adapter and impls; update copyright years
  • Loading branch information
@tjquinno
tjquinno committed Jan 5, 2024
1 parent 217a588 commit bf72b31
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 70 deletions.
9 changes: 1 addition & 8 deletions cors/src/main/java/io/helidon/cors/CorsRequestAdapter.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
* Copyright (c) 2022, 2024 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 @@ -74,13 +74,6 @@ public interface CorsRequestAdapter<T> {
*/
String method();

/**
* Reports whether the request arrived securely (i.e., over https).
*
* @return true if secure
*/
boolean isSecure();

/**
* Processes the next handler/filter/request processor in the chain.
*/
Expand Down
16 changes: 8 additions & 8 deletions cors/src/main/java/io/helidon/cors/CorsSupportHelper.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2023 Oracle and/or its affiliates.
* Copyright (c) 2020, 2024 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 @@ -371,27 +371,27 @@ public Aggregator aggregator() {
}

// For testing
static RequestTypeInfo isRequestTypeNormal(String originHeader, UriInfo requestedHostUri, boolean isRequestSecure) {
return RequestTypeInfo.create(originHeader, requestedHostUri, isRequestSecure);
static RequestTypeInfo isRequestTypeNormal(String originHeader, UriInfo requestedHostUri) {
return RequestTypeInfo.create(originHeader, requestedHostUri);
}

/**
* Captures intermediate data and the final result in deciding whether a request is a normal (non-CORS) request or not.
* <p>
* We want to use the intermediate results for clearer logging if that's turned on without having to recompute it.
* </p>
* @param originLocation full origin (including port)
* @param originLocation full origin (including scheme and port)
* @param hostLocation full host (including scheme and port)
* @param isNormal whether the origin and host information represent a normal (non-CORS) request
*/
record RequestTypeInfo(String originLocation, String hostLocation, boolean isNormal) {

static RequestTypeInfo create(String originHeader, UriInfo requestedHostUri, boolean isRequestSecure) {
static RequestTypeInfo create(String originHeader, UriInfo requestedHostUri) {
String originLocation = CorsSupportHelper.originLocation(originHeader);
String hostLocation = CorsSupportHelper.hostLocation(requestedHostUri);

return new RequestTypeInfo(CorsSupportHelper.originLocation(originHeader),
CorsSupportHelper.hostLocation(requestedHostUri),
return new RequestTypeInfo(originLocation,
hostLocation,
originLocation.equals(hostLocation));
}
}
Expand All @@ -406,7 +406,7 @@ private static boolean isRequestTypeNormal(CorsRequestAdapter<?> requestAdapter,
return true;
}

RequestTypeInfo result = isRequestTypeNormal(originOpt.get(), requestAdapter.requestedUri(), requestAdapter.isSecure());
RequestTypeInfo result = isRequestTypeNormal(originOpt.get(), requestAdapter.requestedUri());
LogHelper.logIsRequestTypeNormal(result.isNormal,
silent,
requestAdapter,
Expand Down
2 changes: 1 addition & 1 deletion cors/src/main/java/io/helidon/cors/LogHelper.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2023 Oracle and/or its affiliates.
* Copyright (c) 2020, 2024 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
54 changes: 17 additions & 37 deletions cors/src/test/java/io/helidon/cors/CorsSupportHelperTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2023 Oracle and/or its affiliates.
* Copyright (c) 2020, 2024 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 @@ -67,76 +67,65 @@ void testNormalize() {
void sameNodeDifferentPorts() {
assertThat("Default different origin port",
CorsSupportHelper.isRequestTypeNormal("http://ok.com",
uriInfo("ok.com", 8010, false),
false).isNormal(),
uriInfo("ok.com", 8010, false)).isNormal(),
is(false));
assertThat("Explicit different origin port",
CorsSupportHelper.isRequestTypeNormal("http://ok.com:8080",
uriInfo("ok.com", false),
false).isNormal(),
uriInfo("ok.com", false)).isNormal(),
is(false));
}

@Test
void sameNodeSamePort() {
assertThat("Default origin port",
CorsSupportHelper.isRequestTypeNormal("http://ok.com",
uriInfo("ok.com", false),
false).isNormal(),
uriInfo("ok.com", false)).isNormal(),
is(true));
assertThat("Explicit origin port",
CorsSupportHelper.isRequestTypeNormal("http://ok.com:80",
UriInfo.builder()
.host("ok.com")
.build(),
false).isNormal(),
.build()).isNormal(),
is(true));
}

@Test
void differentNode() {
assertThat("Different node, same (default) port",
CorsSupportHelper.isRequestTypeNormal("http://bad.com",
uriInfo("ok.com", false),
false).isNormal(),
uriInfo("ok.com", false)).isNormal(),
is(false));
assertThat("Different node, same explicit port",
CorsSupportHelper.isRequestTypeNormal("http://bad.com:80",
uriInfo("ok.com", false),
false).isNormal(),
uriInfo("ok.com", false)).isNormal(),
is(false));

assertThat("Different node, different explicit port",
CorsSupportHelper.isRequestTypeNormal("http://bad.com:8080",
uriInfo("ok.com", false),
false).isNormal(),
uriInfo("ok.com", false)).isNormal(),
is(false));
}

@Test
void differentScheme() {
assertThat("Same node, insecure origin, secure host",
CorsSupportHelper.isRequestTypeNormal("http://foo.com",
uriInfo("foo.com", true),
true).isNormal(),
uriInfo("foo.com", true)).isNormal(),
is(false));

assertThat("Same node, secure origin, insecure host",
CorsSupportHelper.isRequestTypeNormal("https://foo.com",
uriInfo("foo.com", false),
false).isNormal(),
uriInfo("foo.com", false)).isNormal(),
is(false));

assertThat("Different nodes, insecure origin, secure host",
CorsSupportHelper.isRequestTypeNormal("http://foo.com",
uriInfo("other.com", true),
true).isNormal(),
uriInfo("other.com", true)).isNormal(),
is(false));

assertThat("Different nodes, secure origin, insecure host",
CorsSupportHelper.isRequestTypeNormal("https://foo.com",
uriInfo("other.com", false),
false).isNormal(),
uriInfo("other.com", false)).isNormal(),
is(false));
}

Expand All @@ -146,32 +135,27 @@ void sameSecureScheme() {
// secure or not.
assertThat("Same node, secure origin, secure host",
CorsSupportHelper.isRequestTypeNormal("https://foo.com",
uriInfo("foo.com", true),
true).isNormal(),
uriInfo("foo.com", true)).isNormal(),
is(true));

assertThat("Different node, secure origin, secure host",
CorsSupportHelper.isRequestTypeNormal("https://foo.com",
uriInfo("other.com", true),
true).isNormal(),
uriInfo("other.com", true)).isNormal(),
is(false));

assertThat("Same nodes, different ports, secure origin, secure host",
CorsSupportHelper.isRequestTypeNormal("https://foo.com:1234",
uriInfo("foo.com",5678, true),
true).isNormal(),
uriInfo("foo.com",5678, true)).isNormal(),
is(false));

assertThat("Same nodes, explicit origin port, secure origin, secure host",
CorsSupportHelper.isRequestTypeNormal("https://foo.com:443",
uriInfo("foo.com", true),
true).isNormal(),
uriInfo("foo.com", true)).isNormal(),
is(true));

assertThat("Same nodes, explicit host port, secure origin, secure host",
CorsSupportHelper.isRequestTypeNormal("https://foo.com",
uriInfo("foo.com", 443, true),
true).isNormal(),
uriInfo("foo.com", 443, true)).isNormal(),
is(true));
}

Expand Down Expand Up @@ -199,7 +183,6 @@ void checkPlainRequestToPlainPath() {
CorsRequestAdapter<TestCorsServerRequestAdapter> req = new TestCorsServerRequestAdapter("/greet",
uriInfo,
"OPTIONS",
false,
headers);
CorsResponseAdapter<TestCorsServerResponseAdapter> resp = new TestCorsServerResponseAdapter();

Expand All @@ -224,7 +207,6 @@ void checkSecureAccessToPlainPath() {
CorsRequestAdapter<TestCorsServerRequestAdapter> secureReq = new TestCorsServerRequestAdapter("/greet",
uriInfo,
"OPTIONS",
true,
headers);
CorsResponseAdapter<TestCorsServerResponseAdapter> secureResp = new TestCorsServerResponseAdapter();

Expand Down Expand Up @@ -252,7 +234,6 @@ void checkPlainAccessToSecurePath() {
CorsRequestAdapter<TestCorsServerRequestAdapter> secureReq = new TestCorsServerRequestAdapter("/secure-greet",
uriInfo,
"OPTIONS",
false,
headers);
CorsResponseAdapter<TestCorsServerResponseAdapter> secureResp = new TestCorsServerResponseAdapter();

Expand Down Expand Up @@ -280,7 +261,6 @@ void checkSecureAccessToSecurePath() {
CorsRequestAdapter<TestCorsServerRequestAdapter> secureReq = new TestCorsServerRequestAdapter("/secure-greet",
uriInfo,
"OPTIONS",
true,
headers);
CorsResponseAdapter<TestCorsServerResponseAdapter> secureResp = new TestCorsServerResponseAdapter();

Expand Down
4 changes: 2 additions & 2 deletions cors/src/test/java/io/helidon/cors/TestCorsServerRequestAdapter.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates.
* Copyright (c) 2024 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 @@ -22,7 +22,7 @@
import io.helidon.http.HeaderName;
import io.helidon.http.Headers;

record TestCorsServerRequestAdapter(String path, UriInfo requestedUri, String method, boolean isSecure, Headers headers) implements CorsRequestAdapter<TestCorsServerRequestAdapter> {
record TestCorsServerRequestAdapter(String path, UriInfo requestedUri, String method, Headers headers) implements CorsRequestAdapter<TestCorsServerRequestAdapter> {

@Override
public Optional<String> firstHeader(HeaderName key) {
Expand Down
2 changes: 1 addition & 1 deletion cors/src/test/java/io/helidon/cors/TestCorsServerResponseAdapter.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates.
* Copyright (c) 2024 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
2 changes: 1 addition & 1 deletion cors/src/test/resources/configMapperTest.yaml
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2020, 2023 Oracle and/or its affiliates.
# Copyright (c) 2020, 2024 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
7 changes: 1 addition & 6 deletions microprofile/cors/src/main/java/io/helidon/microprofile/cors/CorsSupportMp.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2023 Oracle and/or its affiliates.
* Copyright (c) 2020, 2024 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 @@ -148,11 +148,6 @@ public ContainerRequestContext request() {
return requestContext;
}

@Override
public boolean isSecure() {
return requestContext.getSecurityContext().isSecure();
}

@Override
public void next() {
}
Expand Down
7 changes: 1 addition & 6 deletions webserver/cors/src/main/java/io/helidon/webserver/cors/CorsServerRequestAdapter.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2023 Oracle and/or its affiliates.
* Copyright (c) 2020, 2024 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 @@ -73,11 +73,6 @@ public String method() {
return request.prologue().method().text();
}

@Override
public boolean isSecure() {
return request.isSecure();
}

@Override
public void next() {
response.next();
Expand Down

0 comments on commit bf72b31

Please sign in to comment.