Corsfilter fix by beepsoft · Pull Request #12151 · IQSS/dataverse

beepsoft · 2026-02-06T12:56:51Z

What this PR does / why we need it:

Fixes CorsFilter invocation inconsistency.

When accessing /api/... endpoints the CorsFilter was not always hit and so these endpoints could not be used from webapps even if CORS was properly configured.

The problem seemed to be with ApiRouter's forwarding mechanism from /api/... to /api/v1/...

Adding FORWARD dispatcher types to @WebFilter solves this.

Fix is based on this zulip thread:

https://dataverse.zulipchat.com/#narrow/channel/379673-dev/topic/CorsFilter.20Servlet.20Filter.20not.20discovered/near/572358009

Suggestions on how to test this:

CorsIT.java is added to test whether CORS headers are present for both /api/... and /api/v1/... invocations. For it to pass CORS must be enabled in Dataverse, eg. by setting

DATAVERSE_CORS_ORIGIN: "*"

env for dev_dataverse in docker-compose-dev.yml

@webfilter

When accessing /api/... endpoints the CorsFilter was not always hit and so these endpoints could not be used from webapps even if CORS was properly configured. The problem seemed to be with ApiRouter's forwarding mechanism from /api/... to /api/v1/... Adding FORWARD dispatcher types to @webfilter solves this. Fix is based on this zulip thread: https://dataverse.zulipchat.com/#narrow/channel/379673-dev/topic/CorsFilter.20Servlet.20Filter.20not.20discovered/near/572358009

coveralls · 2026-02-06T13:10:48Z

coverage: 24.323%. remained the same
when pulling 36c336e on beepsoft:cors-filter-fix
into 86796a1 on IQSS:develop.

poikilotherm · 2026-02-06T14:12:10Z

src/test/java/edu/harvard/iq/dataverse/api/CorsIT.java

+    }
+
+    @ParameterizedTest(name = "CORS preflight headers on {0}")
+    @MethodSource("preflightEndpoints")


It seems a bit extra to use a MethodSource here. A ValueSource with the strings should be fine.

I took it from another random ...IT.java but I see @MethodSource and @valuesource are used quite randomly, but I updated to @valuesource.

poikilotherm · 2026-02-06T14:13:32Z

src/test/java/edu/harvard/iq/dataverse/api/CorsIT.java

+            .header("User-Agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/144.0.0.0 Safari/537.36")
+            .options(path);
+
+        int statusCode = response.getStatusCode();


While this is technically OK, I'd rather see adoption of the assertions built into RestAssured. That way we can leverage its logging functionality as well.

poikilotherm · 2026-02-06T14:28:40Z

src/main/java/edu/harvard/iq/dataverse/filter/CorsFilter.java

 */

-@WebFilter("/*")
+@WebFilter(value = "/*", dispatcherTypes = { DispatcherType.REQUEST, DispatcherType.FORWARD})


We should add dispatch types "ERROR" and "ASYNC" here, too. Error to make really sure the error pages are handled correctly with CORS, Async for future extensions to the JAX-RS API to return Futures.

"INCLUDE" is probably not necessary here, as it would be used internally anyway. No way an outside request would have an INCLUDE dispatch type.

I also strongly suggest to add detailed explanation to the Javadoc comment, linking to ApiRouter etc. Otherwise this context may be forgotten again.

Added those and updated javadoc.

pdurbin

A quick review.

pdurbin · 2026-02-06T15:03:19Z

src/main/java/edu/harvard/iq/dataverse/filter/CorsFilter.java

@@ -9,6 +9,7 @@

 import edu.harvard.iq.dataverse.settings.JvmSettings;


I'm just going to put this comment at the top but can you please add a release not snippet that describes the bug being fixed? Thanks! I think you know the drill, but just in case: https://guides.dataverse.org/en/6.9/developers/version-control.html#writing-a-release-note-snippet

ErykKul

Looks good to me. Thanks! Just a release note and it's ready.

beepsoft · 2026-02-09T09:27:41Z

@ErykKul Should I just add doc/release-notes/cors-filter-fix.md? I don't have an associated issue and so my branch name is not prefixed with the issue number, which seems to be required by the doc/release-notes/README.md

poikilotherm · 2026-02-11T21:08:36Z

@beepsoft Please feel free to create an issue and link this PR to it. Can be brief.

Also, I see Jenkins is not happy with one IT: [ERROR] Failures: ", "[ERROR] SwordIT.testDeleteFiles:961 1 expectation failed.", "Expected status code <200> but was <500>."

Can you please look into that? Thx!

Also, CorsIT must be added to https://github.com/IQSS/dataverse/blob/develop/tests/integration-tests.txt

beepsoft · 2026-02-12T13:47:50Z

Do we need an issue at this point? I only raised the issue question because the release note authoring README.md mentioned, but I don't know if and how that applies here.

Regarding SwordIT: I could not reproduce it locally.

I added CorsIT to integration-tests.txt.

tests/integration-tests.txt

Co-authored-by: Oliver Bertuch <poikilotherm@users.noreply.github.com>

poikilotherm · 2026-02-13T14:17:28Z

I see in https://jenkins.dataverse.org/blue/organizations/jenkins/IQSS-Dataverse-Develop-PR/detail/PR-12151/7/tests that the tests now run, but CorsIT fails - the headers are all not present.

I have a hunch that we might need @donsizemore to enable CORS during testing. Unfortunately, we currently have no way to change JVM options at runtime (there may exist an issue about this already).

@pdurbin WDYT should we disable these tests until we figure out the remote JVM option part? Can we just enable CORS in Jenkins without affecting other tests?

qqmyers · 2026-02-13T14:23:06Z

FWIW: I've recently started making non-IT tests with direct (mocked) calls to API methods to be able to test non-default JVM options, e.g. in LDNInboxTest.java and LocalContextsTest.java. Is that a useful approach here?

beepsoft added 3 commits February 6, 2026 13:23

Fix imports

ccfe364

Add CorsIT for minimal CORS header checking

860bd51

beepsoft added 2 commits February 6, 2026 14:39

Make it ParameterizedTest, add more endpoints

c7c3c68

Fix code style

140a799

poikilotherm reviewed Feb 6, 2026

View reviewed changes

poikilotherm added this to Forschungszentrum Jülich and IQSS Dataverse Project Feb 6, 2026

poikilotherm moved this to WIP in Forschungszentrum Jülich Feb 6, 2026

poikilotherm requested review from ErykKul and qqmyers February 6, 2026 14:46

poikilotherm marked this pull request as draft February 6, 2026 14:46

pdurbin reviewed Feb 6, 2026

View reviewed changes

beepsoft added 2 commits February 6, 2026 17:28

Add ERROR and ASYNC dispatcher type and more docs

81a4e32

Use idiomatic RestAssured assertions

36c336e

ErykKul approved these changes Feb 9, 2026

View reviewed changes

cmbz added the FY26 Sprint 16 FY26 Sprint 16 (2026-01-28 - 2026-02-11) label Feb 11, 2026

poikilotherm moved this from WIP to Important in Forschungszentrum Jülich Feb 11, 2026

cmbz added the FY26 Sprint 17 FY26 Sprint 17 (2026-02-11 - 2026-02-25) label Feb 11, 2026

beepsoft added 2 commits February 12, 2026 14:39

Merge branch 'IQSS:develop' into cors-filter-fix

3b117c1

Add CorsIT to integration-tests.txt

2a85be7

poikilotherm reviewed Feb 13, 2026

View reviewed changes

tests/integration-tests.txt Outdated Show resolved Hide resolved

Update tests/integration-tests.txt

5b74dd0

Co-authored-by: Oliver Bertuch <poikilotherm@users.noreply.github.com>

		@@ -9,6 +9,7 @@

		import edu.harvard.iq.dataverse.settings.JvmSettings;

Conversation

beepsoft commented Feb 6, 2026 • edited by pdurbin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poikilotherm Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

beepsoft Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

poikilotherm Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beepsoft Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

poikilotherm Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

poikilotherm Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beepsoft Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

pdurbin left a comment

Choose a reason for hiding this comment

Uh oh!

pdurbin Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

ErykKul left a comment

Choose a reason for hiding this comment

Uh oh!

beepsoft commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poikilotherm commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

beepsoft commented Feb 12, 2026

Uh oh!

Uh oh!

poikilotherm commented Feb 13, 2026

Uh oh!

qqmyers commented Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

beepsoft commented Feb 6, 2026 •

edited by pdurbin

Loading

coveralls commented Feb 6, 2026 •

edited

Loading

poikilotherm Feb 6, 2026 •

edited

Loading

poikilotherm Feb 6, 2026 •

edited

Loading

beepsoft commented Feb 9, 2026 •

edited

Loading

poikilotherm commented Feb 11, 2026 •

edited

Loading