Skip to content

Commit

Permalink
fix(graphql) this fixes the preflight OPTIONS call that was throwing … (
Browse files Browse the repository at this point in the history
#30299)

This pull request fixes the OPTIONS method that is called in pre-flight
request and was failing with a 400

It also includes several changes to the `DotGraphQLHttpServlet` class
and its corresponding test class to improve CORS header handling and
update the usage of the `vavr` library. The most important changes
include replacing `Function0` with `Lazy` for CORS headers, updating
method calls to use `get()` instead of `apply()`, and modifying the
configuration retrieval logic.

### Improvements to CORS header handling:

*
[`dotCMS/src/main/java/com/dotcms/graphql/DotGraphQLHttpServlet.java`](diffhunk://#diff-2cf65c77c46e085836ba6fe1ec0fb24afc2226d1e854e68fed23319d2e7306f2L9-R10):
Replaced the `Function0` type with `Lazy` for the `corsHeaders` field to
improve lazy initialization and readability.
[[1]](diffhunk://#diff-2cf65c77c46e085836ba6fe1ec0fb24afc2226d1e854e68fed23319d2e7306f2L9-R10)
[[2]](diffhunk://#diff-2cf65c77c46e085836ba6fe1ec0fb24afc2226d1e854e68fed23319d2e7306f2L67-R94)
*
[`dotCMS/src/main/java/com/dotcms/graphql/DotGraphQLHttpServlet.java`](diffhunk://#diff-2cf65c77c46e085836ba6fe1ec0fb24afc2226d1e854e68fed23319d2e7306f2L42-R44):
Updated the `handleRequest` and `doOptions` methods to use
`corsHeaders.get()` instead of `corsHeaders.apply()`.
[[1]](diffhunk://#diff-2cf65c77c46e085836ba6fe1ec0fb24afc2226d1e854e68fed23319d2e7306f2L42-R44)
[[2]](diffhunk://#diff-2cf65c77c46e085836ba6fe1ec0fb24afc2226d1e854e68fed23319d2e7306f2L51-R53)

### Configuration retrieval logic:

*
[`dotCMS/src/main/java/com/dotcms/graphql/DotGraphQLHttpServlet.java`](diffhunk://#diff-2cf65c77c46e085836ba6fe1ec0fb24afc2226d1e854e68fed23319d2e7306f2L67-R94):
Modified the configuration retrieval logic to use
`Config.subsetContainsAsList` and filter keys starting with specific
prefixes for better clarity and efficiency.

### Test updates:

*
[`dotcms-integration/src/test/java/com/dotcms/graphql/DotGraphQLHttpServletTest.java`](diffhunk://#diff-fd7d80a1699d72b55de4c755c51f3a67c186e7855dc00ea2c383f0d6a38fee2fL27-R27):
Updated the test method `testing_cors_headers` to use
`corsHeaders.get()` instead of `corsHeaders.apply()`.
  • Loading branch information
wezell authored Oct 8, 2024
1 parent 130efbc commit 0075d70
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 15 deletions.
28 changes: 14 additions & 14 deletions dotCMS/src/main/java/com/dotcms/graphql/DotGraphQLHttpServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
import com.dotmarketing.util.UtilMethods;
import graphql.kickstart.servlet.AbstractGraphQLHttpServlet;
import graphql.kickstart.servlet.GraphQLConfiguration;
import io.vavr.Function0;
import io.vavr.Lazy;
import io.vavr.control.Try;
import java.util.HashMap;
import java.util.List;
import javax.servlet.http.HttpServletRequest;
Expand Down Expand Up @@ -39,7 +40,8 @@ protected void doPost(final HttpServletRequest request, final HttpServletRespons

@Override
protected void doOptions(final HttpServletRequest request, final HttpServletResponse response) {
handleRequest(request, response);
corsHeaders.get().forEach(response::setHeader);

}

/**
Expand All @@ -48,7 +50,7 @@ protected void doOptions(final HttpServletRequest request, final HttpServletResp
* @param response
*/
protected void handleRequest(HttpServletRequest request, HttpServletResponse response) {
corsHeaders.apply().forEach(response::setHeader);
corsHeaders.get().forEach(response::setHeader);
try {
getConfiguration().getHttpRequestHandler().handle(request, response);
} catch (Exception t) {
Expand All @@ -64,38 +66,36 @@ protected void handleRequest(HttpServletRequest request, HttpServletResponse res
* those with the specific ones for graphql, api.cors.graphql.access-control-allow-headers
*
*/
protected final Function0<HashMap<String, String>> corsHeaders = Function0.of(() -> {
protected final Lazy<HashMap<String, String>> corsHeaders = Lazy.of(() -> {

final HashMap<String, String> headers = new HashMap<>();
// load defaults
Config.subset(CORS_DEFAULT).forEachRemaining(k -> {
final String prop = Config.getStringProperty(CORS_DEFAULT + "." + k, null);
Config.subsetContainsAsList(CORS_DEFAULT).stream().filter(k -> k.startsWith(CORS_DEFAULT)).forEach(k -> {
final String prop = Config.getStringProperty(k, null);
if (UtilMethods.isSet(prop)) {
headers.put(k.toLowerCase(), prop);
headers.put(k.replace(CORS_DEFAULT + ".", "").toLowerCase(), prop);
}

});
// then override with graph
Config.subset(CORS_GRAPHQL).forEachRemaining(k -> {
final String prop = Config.getStringProperty(CORS_GRAPHQL + "." + k, null);
Config.subsetContainsAsList(CORS_GRAPHQL).stream().filter(k -> k.startsWith(CORS_GRAPHQL)).forEach(k -> {
final String prop = Config.getStringProperty(k, null);
if (UtilMethods.isSet(prop)) {
headers.put(k.toLowerCase(), prop);
headers.put(k.replace(CORS_GRAPHQL + ".", "").toLowerCase(), prop);
} else {
headers.remove(k.toLowerCase());
}

});



return headers;


}).memoized();
});






}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static void prepare() throws Exception{
@Test
public void testing_cors_headers() {

HashMap<String,String> headers = new DotGraphQLHttpServlet().corsHeaders.apply();
HashMap<String,String> headers = new DotGraphQLHttpServlet().corsHeaders.get();

assertEquals(headers.get("access-control-allow-origin"), "*");
assertEquals(headers.get("access-control-allow-credentials"), "true");
Expand Down

0 comments on commit 0075d70

Please sign in to comment.