Skip to content

Corsfilter fix#12151

Open
beepsoft wants to merge 21 commits intoIQSS:developfrom
beepsoft:cors-filter-fix
Open

Corsfilter fix#12151
beepsoft wants to merge 21 commits intoIQSS:developfrom
beepsoft:cors-filter-fix

Conversation

@beepsoft
Copy link
Contributor

@beepsoft beepsoft commented Feb 6, 2026 •
edited by poikilotherm
Loading

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

Also fixes #12161 with docs changes made by @poikilotherm originally as PR #12162

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

@beepsoft
Fix CorsFilter invocation inconsistency
e7e9f18 
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
@beepsoft
Fix imports
ccfe364
@beepsoft
Add CorsIT for minimal CORS header checking
860bd51
@coveralls
Copy link

coveralls commented Feb 6, 2026 •
edited
Loading

Coverage Status

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

poikilotherm
poikilotherm reviewed Feb 6, 2026
View reviewed changes
poikilotherm
poikilotherm reviewed Feb 6, 2026
View reviewed changes
poikilotherm
poikilotherm reviewed Feb 6, 2026
View reviewed changes
@poikilotherm poikilotherm added Feature: API Component: Code Infrastructure formerly "Feature: Code Infrastructure" Feature: Permissions Component: JSF Involves modifying JSF (Jakarta Server Faces) code, which is being replaced with React. Size: 3 A percentage of a sprint. 2.1 hours. SPA These changes are required for the Dataverse SPA labels Feb 6, 2026
@poikilotherm poikilotherm marked this pull request as draft February 6, 2026 14:46
pdurbin
pdurbin reviewed Feb 6, 2026
View reviewed changes
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quick review.

ErykKul
ErykKul approved these changes Feb 9, 2026
View reviewed changes
Copy link
Collaborator

@ErykKul ErykKul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@beepsoft
Copy link
Contributor Author

beepsoft commented Feb 9, 2026 •
edited
Loading

@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

@cmbz cmbz added the FY26 Sprint 16 FY26 Sprint 16 (2026-01-28 - 2026-02-11) label Feb 11, 2026
@poikilotherm
Copy link
Contributor

poikilotherm commented Feb 11, 2026 •
edited
Loading

@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

@poikilotherm poikilotherm moved this from WIP to Important in Forschungszentrum Jülich Feb 11, 2026
@cmbz cmbz added the FY26 Sprint 17 FY26 Sprint 17 (2026-02-11 - 2026-02-25) label Feb 11, 2026
@poikilotherm
Copy link
Contributor

poikilotherm commented Feb 13, 2026

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
Copy link
Member

qqmyers commented Feb 13, 2026

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?

@pdurbin pdurbin moved this from Ready for QA ⏩ to Ready for Review ⏩ in IQSS Dataverse Project Feb 17, 2026
@pdurbin
Copy link
Member

pdurbin commented Feb 17, 2026

There seem to be some open questions and some failing tests and the PR is still in draft so I moved it from "ready for QA" to "ready for review".

@pdurbin
Copy link
Member

pdurbin commented Feb 24, 2026

@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

@beepsoft doc/release-notes/cors-filter-fix.md is fine. Also, please click "ready for review" (if you're ready), to take this PR out of draft.

@pdurbin pdurbin moved this from Ready for Review ⏩ to In Progress 💻 in IQSS Dataverse Project Feb 24, 2026
@beepsoft beepsoft marked this pull request as ready for review February 25, 2026 06:30
@pdurbin pdurbin moved this from In Progress 💻 to Ready for Review ⏩ in IQSS Dataverse Project Feb 25, 2026
@cmbz cmbz added the FY26 Sprint 18 FY26 Sprint 18 (2026-02-25 - 2026-03-11) label Feb 26, 2026
@poikilotherm
Copy link
Contributor

poikilotherm commented Feb 26, 2026

@pdurbin do you want me to make #12162 a part of this PR?

@pdurbin
Copy link
Member

pdurbin commented Mar 4, 2026

@beepsoft what do you think?

@beepsoft
Copy link
Contributor Author

beepsoft commented Mar 4, 2026

If it makes sense, sure! Do I have to do anything about it?

@pdurbin
Copy link
Member

pdurbin commented Mar 4, 2026

"Maintainers are allowed to edit this pull request" and @poikilotherm is in that list. So he can go ahead. 🎉

@pdurbin pdurbin moved this from Ready for Review ⏩ to In Progress 💻 in IQSS Dataverse Project Mar 4, 2026
@poikilotherm
Merge branch 'develop' into cors-filter-fix
f2bb1c0
@poikilotherm
docs: move big-data-support from developers to installation guide IQS…
992acb1 
…S#12161

Relocated the big-data-support documentation to the installation section and updated all cross-references accordingly. Added redirect for the old location.
@poikilotherm
docs(installation): expand CORS documentation and reorganize settings…
4d8b6fd 
… section IQSS#12161
@poikilotherm
docs: clarify and relocate general CORS documentation for external to…
ac0669c 
…ols and S3 admin guide

Updated external tools documentation to clarify CORS requirements and renamed reference to `:ref:`dataverse.cors``.
Changed CORS section in big-data-administration.rst to use explicit cross-reference label `:ref:`cors-s3-bucket`` instead of textual reference only.
@poikilotherm
docs,style(installation): clarify CORS options default behavior and e…
d85fd3d 
…xpand configuration option descriptions
@poikilotherm
docs(installation): expand and reorganize S3 CORS documentation IQSS#…
e8a3403 
…12161

- Clarify CORS requirements for direct uploads and downloads
- Add cross-references to Dataverse CORS settings
- Provide step-by-step instructions and examples for AWS CLI and Minio
- Refactor guidance on ensuring origin compatibility between Dataverse and S3
@poikilotherm
chore(docs): update Read the Docs configuration to Ubuntu 24.04 and P…
ff86b9b 
…ython 3.12 IQSS#12161

Necessary update for RTD and to enable using sphinx-reredirect extension
@poikilotherm
Merge branch '12161-extend-cors-docs' into cors-filter-fix
60a3541
@poikilotherm
Copy link
Contributor

poikilotherm commented Mar 5, 2026

I merged the latest develop into this branch and merged my docs branch into it as well.

@poikilotherm poikilotherm mentioned this pull request Mar 5, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pdurbin pdurbin pdurbin left review comments

@poikilotherm poikilotherm poikilotherm left review comments

@ErykKul ErykKul ErykKul approved these changes

@qqmyers qqmyers Awaiting requested review from qqmyers

Assignees

@poikilotherm poikilotherm

Labels

Component: Code Infrastructure formerly "Feature: Code Infrastructure" Component: JSF Involves modifying JSF (Jakarta Server Faces) code, which is being replaced with React. Feature: API Feature: Permissions FY26 Sprint 16 FY26 Sprint 16 (2026-01-28 - 2026-02-11) FY26 Sprint 17 FY26 Sprint 17 (2026-02-11 - 2026-02-25) FY26 Sprint 18 FY26 Sprint 18 (2026-02-25 - 2026-03-11) Size: 3 A percentage of a sprint. 2.1 hours. SPA These changes are required for the Dataverse SPA

Projects

Forschungszentrum Jülich
Status: Important
IQSS Dataverse Project
Status: In Progress 💻

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Extend CORS documentation

7 participants

@beepsoft @coveralls @poikilotherm @qqmyers @pdurbin @ErykKul @cmbz