Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@tjquinno
tjquinno committed Jan 5, 2024
1 parent bf72b31 commit c4b867b
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 40 deletions.
4 changes: 2 additions & 2 deletions cors/src/main/java/io/helidon/cors/CorsSupportHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ public Aggregator aggregator() {
}

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

Expand Down Expand Up @@ -406,7 +406,7 @@ private static boolean isRequestTypeNormal(CorsRequestAdapter<?> requestAdapter,
return true;
}

RequestTypeInfo result = isRequestTypeNormal(originOpt.get(), requestAdapter.requestedUri());
RequestTypeInfo result = requestType(originOpt.get(), requestAdapter.requestedUri());
LogHelper.logIsRequestTypeNormal(result.isNormal,
silent,
requestAdapter,
Expand Down
10 changes: 4 additions & 6 deletions cors/src/main/java/io/helidon/cors/LogHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,10 @@ static <T> void logIsRequestTypeNormalNoOrigin(boolean silent, CorsRequestAdapte
if (silent || !CorsSupportHelper.LOGGER.isLoggable(DECISION_LEVEL)) {
return;
}
if (CorsSupportHelper.LOGGER.isLoggable(DECISION_LEVEL)) {
CorsSupportHelper.LOGGER.log(DECISION_LEVEL,
String.format("Request %s is not cross-host: %s",
requestAdapter,
List.of("header " + HeaderNames.ORIGIN + " is absent")));
}
CorsSupportHelper.LOGGER.log(DECISION_LEVEL,
String.format("Request %s is not cross-host: %s",
requestAdapter,
List.of("header " + HeaderNames.ORIGIN + " is absent")));
}

static <T> void logIsRequestTypeNormal(boolean result, boolean silent, CorsRequestAdapter<T> requestAdapter,
Expand Down
64 changes: 32 additions & 32 deletions cors/src/test/java/io/helidon/cors/CorsSupportHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,24 +66,24 @@ void testNormalize() {
@Test
void sameNodeDifferentPorts() {
assertThat("Default different origin port",
CorsSupportHelper.isRequestTypeNormal("http://ok.com",
uriInfo("ok.com", 8010, false)).isNormal(),
CorsSupportHelper.requestType("http://ok.com",
uriInfo("ok.com", 8010, false)).isNormal(),
is(false));
assertThat("Explicit different origin port",
CorsSupportHelper.isRequestTypeNormal("http://ok.com:8080",
uriInfo("ok.com", false)).isNormal(),
CorsSupportHelper.requestType("http://ok.com:8080",
uriInfo("ok.com", false)).isNormal(),
is(false));
}

@Test
void sameNodeSamePort() {
assertThat("Default origin port",
CorsSupportHelper.isRequestTypeNormal("http://ok.com",
uriInfo("ok.com", false)).isNormal(),
CorsSupportHelper.requestType("http://ok.com",
uriInfo("ok.com", false)).isNormal(),
is(true));
assertThat("Explicit origin port",
CorsSupportHelper.isRequestTypeNormal("http://ok.com:80",
UriInfo.builder()
CorsSupportHelper.requestType("http://ok.com:80",
UriInfo.builder()
.host("ok.com")
.build()).isNormal(),
is(true));
Expand All @@ -92,40 +92,40 @@ void sameNodeSamePort() {
@Test
void differentNode() {
assertThat("Different node, same (default) port",
CorsSupportHelper.isRequestTypeNormal("http://bad.com",
uriInfo("ok.com", false)).isNormal(),
CorsSupportHelper.requestType("http://bad.com",
uriInfo("ok.com", false)).isNormal(),
is(false));
assertThat("Different node, same explicit port",
CorsSupportHelper.isRequestTypeNormal("http://bad.com:80",
uriInfo("ok.com", false)).isNormal(),
CorsSupportHelper.requestType("http://bad.com:80",
uriInfo("ok.com", false)).isNormal(),
is(false));

assertThat("Different node, different explicit port",
CorsSupportHelper.isRequestTypeNormal("http://bad.com:8080",
uriInfo("ok.com", false)).isNormal(),
CorsSupportHelper.requestType("http://bad.com:8080",
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)).isNormal(),
CorsSupportHelper.requestType("http://foo.com",
uriInfo("foo.com", true)).isNormal(),
is(false));

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

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

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

Expand All @@ -134,28 +134,28 @@ void sameSecureScheme() {
// Note that the real UriInfo instances from real requests will set the port according to whether the request is
// secure or not.
assertThat("Same node, secure origin, secure host",
CorsSupportHelper.isRequestTypeNormal("https://foo.com",
uriInfo("foo.com", true)).isNormal(),
CorsSupportHelper.requestType("https://foo.com",
uriInfo("foo.com", true)).isNormal(),
is(true));

assertThat("Different node, secure origin, secure host",
CorsSupportHelper.isRequestTypeNormal("https://foo.com",
uriInfo("other.com", true)).isNormal(),
CorsSupportHelper.requestType("https://foo.com",
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)).isNormal(),
CorsSupportHelper.requestType("https://foo.com:1234",
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)).isNormal(),
CorsSupportHelper.requestType("https://foo.com:443",
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)).isNormal(),
CorsSupportHelper.requestType("https://foo.com",
uriInfo("foo.com", 443, true)).isNormal(),
is(true));
}

Expand Down

0 comments on commit c4b867b

Please sign in to comment.