Balázs Pataki (Feb 05 2026 at 10:30): Balázs Pataki (Feb 05 2026 at 10:30):I'm not sure if it is container related, however it seems that the CorsFilter is not picked up automatically although the @WebFilter("/*") should do that:
@WebFilter("/*")
public class CorsFilter implements Filter
Instead I had to manually add to web.xml this:
<filter>
<filter-name>edu.harvard.iq.dataverse.filter.CorsFilter</filter-name>
<filter-class>edu.harvard.iq.dataverse.filter.CorsFilter</filter-class>
</filter>
<filter-mapping>
<filter-name>edu.harvard.iq.dataverse.filter.CorsFilter</filter-name>
<url-pattern>/*</url-pattern>
</filter-mapping>
I also checked in the debugger and without the explicit web.xml doFilter() is not get called.
Notification Bot (Feb 05 2026 at 13:12):This topic was moved here from #containers > CorsFilter problem by Oliver Bertuch.
Oliver Bertuch (Feb 05 2026 at 13:12):As this is not purely container related, I moved this here @Balázs Pataki
Oliver Bertuch (Feb 05 2026 at 13:13):Are there any API tests for the filter?
Oliver Bertuch (Feb 05 2026 at 13:13):It would be good to have a reproducer.
Balázs Pataki (Feb 05 2026 at 13:14):I just tried calling a DV endpoint from a browser component, but try to come up with a curl.
Oliver Bertuch (Feb 05 2026 at 13:25):It would be good to have an API test included in our regular suite, so we can also test this is in CI
Balázs Pataki (Feb 05 2026 at 16:39):Actually, I don't know how to make curl fail with CORS, because it never does as it doesn't care, only browsers do, right? For now I just vibe coded a simple single page app which tries to create a dataset with POST and it does fail for me with stock develop branch DV (without manually adding the filter to webxml):
Just open it in a browser, set the api token and press "POST JSON" to see if it fails or not. (It uses the dataset-finch1.json from the DV API guides)
Oliver Bertuch (Feb 05 2026 at 18:23):I was suggesting creating an IT test that checks for presence of the headers.
Oliver Bertuch (Feb 05 2026 at 18:24):(As you said - anything that's not a browser won't care. But we'd only want to verify the servlet filter is picked up)
Balázs Pataki (Feb 06 2026 at 10:13):Ok, I created a CorsIT test checking for CORS headers.
It turns out that the result is kind of random based on whether the server happens to pick up CorsFilter or not.
Here's my actual test session: I run the app in docker. When it is started I run the test manually. Then I stop the server with ctrl+c and either start it again with run, or first do an explicit stop. None of these seem to matter, the result seems to be random:
mvn -Pct docker:stop
mvn -Pct docker:run --> FAIL
ctrl+c
mvn -Pct docker:run --> FAIL
ctrl+c
mvn -Pct docker:stop
mvn -Pct docker:run --> PASS
ctrl+c
mvn -Pct docker:run --> PASS
ctrl+c
mvn -Pct docker:run --> FAIL
I don't know how to proceed from here. One thing is sure: If add the explicit web.xml filter and filter-mapping it works consistently.
Balázs Pataki (Feb 06 2026 at 10:40):This is the test I run:
In the docker-compose-dev.yml I set these for dev_dataverse just to see if I get specific headers I configure:
DATAVERSE_CORS_ORIGIN: "*"
DATAVERSE_CORS_HEADERS_ALLOW: "Accept,Content-Type,X-Dataverse-key,Authorization,cedar-client-session-id,cedar-debug"
``
In your test I see that you are targeting an API endpoint.
Balázs Pataki (Feb 06 2026 at 11:07):Yes, /api/dataverses/root/datasets
Oliver Bertuch (Feb 06 2026 at 11:07):Are we sure the filter actually will correctly work with JAX-RS
Oliver Bertuch (Feb 06 2026 at 11:07):What happens when hitting JSF or other pure servlets?
Balázs Pataki (Feb 06 2026 at 11:08):Yes, sometimes it does, sometimes it doesn't. I don't know what kind of race condition could
it be.
Oliver Bertuch (Feb 06 2026 at 11:08):What happens if we hit a 404 in a servlet
Balázs Pataki (Feb 06 2026 at 11:08):I don't know about JSF, but I want to access api endpoints from a webapp
Balázs Pataki (Feb 06 2026 at 11:09):What url do you suggest to test?
Oliver Bertuch (Feb 06 2026 at 11:12):Technically, JAX-RS is still a servlet, but it's special. We may be in trouble here due to filter ordering etc.
Oliver Bertuch (Feb 06 2026 at 11:12):Also, AI tells me there may be a problem with the mixed style of having web.xml registered filters and annotation based. Not sure if this really is a thing, but lets keep it in mind.
Balázs Pataki (Feb 06 2026 at 11:13):Yes, I think it definitely affects it, although as I read about it should be still deterministic
Balázs Pataki (Feb 06 2026 at 11:15):SwordFilter is the only one configured in web.xml. Maybe removing it from there and adding @WebFilter annotation on the class makes it clear.
Balázs Pataki (Feb 06 2026 at 11:15):I try this.
Oliver Bertuch (Feb 06 2026 at 11:17):Technically for JAX-RS we should be using @ContainerRequestFilter to be within the consistent filter framework of JAX-RS
Oliver Bertuch (Feb 06 2026 at 11:18):See ApiBlockingFilter in api.filter package.
Oliver Bertuch (Feb 06 2026 at 11:18):So this may be another culprit, why things my go sideways - the servlet filter and the JAX-RS filter may conflict.
Oliver Bertuch (Feb 06 2026 at 11:19):At least if both are discovered by annotations scanning.
Oliver Bertuch (Feb 06 2026 at 11:19):Maybe making the filter explicit in web.xml makes it load before the other or sth
Oliver Bertuch (Feb 06 2026 at 11:24):Hmm coming to think about it: any servlet filters will be applied first, then the JAX-RS internal filters will apply. These two don't mix-n-match, they are at different layers. So should be nothing to worry about here.
Oliver Bertuch (Feb 06 2026 at 11:27):@Balázs Pataki could you please verify something for me...
Balázs Pataki (Feb 06 2026 at 11:27):Yes, sure.
Oliver Bertuch (Feb 06 2026 at 11:27):Could you run the tests against /api/v1/...
Oliver Bertuch (Feb 06 2026 at 11:28):I had a look at all the filters we already have and ApiRouter popped up. Asking AI about it gave me this:
The
ApiRouteris doing an internalRequestDispatcher.forward(...)from:
/api/...→/api/v1/...(your JAX‑RS app is mounted at )@ApplicationPath("api/v1")That has a big side effect in Servlet-land: a forward is a second dispatch with dispatcher type FORWARD, and many servlet filters only apply to dispatcher type REQUEST unless explicitly configured.
Oliver Bertuch (Feb 06 2026 at 11:30):So let's rule out that the router may mess up our filter chain
Balázs Pataki (Feb 06 2026 at 11:34):In the meantime I tested putting @WebFilter onto SwordFilter and removing the web.xml config --> same random test behaviour
I now try the /api/v1 behaviour
Oliver Bertuch (Feb 06 2026 at 11:35):Great to verify that! We doubted it would be related, glad to see it was indeed not.
Oliver Bertuch (Feb 06 2026 at 11:36):My money is on Router vs Cors, as both affect the same endpoints.
Oliver Bertuch (Feb 06 2026 at 11:37):Also, you might want to do the testing with the sword filter as well :wink: Not sure if the order matters with those two. IMHO filters should avoid being dependent on order. But of course we'd need to take care of types (forward, request)
Balázs Pataki (Feb 06 2026 at 11:45):Yep. So, when /api/dataverses/root/datasets fails /api/v1/dataverses/root/datasets passes the test.
Balázs Pataki (Feb 06 2026 at 11:46):Now what? :thinking:
Balázs Pataki (Feb 06 2026 at 11:50):In case of /api/v1 it first reaches CorsFilter then ApiBlockingFilter
In case of /api only reaches ApiBlockingFilter
(doFilter(), I mean)
Oliver Bertuch (Feb 06 2026 at 11:51):Ha! So Api Router is the culprit
Oliver Bertuch (Feb 06 2026 at 11:53):But you're somewhat wrong. The chain is:
/api -> Router -> forward -> Cors -> ApiBlocking (which breaks at Cors because forward)
/api/v1 -> Router -> Cors -> ApiBlocking
Balázs Pataki (Feb 06 2026 at 11:54):Maybe that's how it should be, but no, in case of /api Cors is not ALWAYS reached.
Balázs Pataki (Feb 06 2026 at 11:55):So in case of
RequestDispatcher dsp = request.getRequestDispatcher(newRequestUri);
dsp.forward(req, sr1);
wherever it goes it won't always goes thro CorsFilter, it is not in that filter chain, or sg.
Oliver Bertuch (Feb 06 2026 at 11:55):So what we need to do: add both "FORWARD" and "REQUEST" to CorsFilter:
@WebFilter(value = "/*", dispatcherTypes = {DispatcherType.REQUEST, DispatcherType.FORWARD})
We should also look at the types ERROR and INCLUDE later on to avoid breaking CORS when serving error pages etc
Oliver Bertuch (Feb 06 2026 at 11:56):Balázs Pataki said:
Maybe that's how it should be, but no, in case of /api Cors is not ALWAYS reached.
It goes there, but will not match because of dispatcher type forward!
Balázs Pataki (Feb 06 2026 at 11:56):Ok, I check now adding DispatcherType.FORWARD
Oliver Bertuch (Feb 06 2026 at 11:57):Please make sure to use the above example, otherwise we'd break REQUEST :wink:
Balázs Pataki (Feb 06 2026 at 11:57):yeah, i copy pasted that :-)
Balázs Pataki (Feb 06 2026 at 12:06):Oh yeah, it works consistently now! :tada:
And the filter orders are as follows:
/api
cors -> router -> cors -> apiblocking
/api/v1
cors -> router -> apiblocking
Philip Durbin 🚀 (Feb 06 2026 at 12:30):Good morning. :coffee: You've been busy!
Balázs Pataki (Feb 06 2026 at 12:57):Submitted a PR:
https://github.com/IQSS/dataverse/pull/12151
Should we add anything else to it, @Oliver Bertuch ?
Oliver Bertuch (Feb 06 2026 at 12:58):Can you test the error pages?
Oliver Bertuch (Feb 06 2026 at 12:58):And SWORD maybe?
Balázs Pataki (Feb 06 2026 at 12:59):You mean to add more URL-s to test in CorsIT, right?
Oliver Bertuch (Feb 06 2026 at 12:59):Yeah, lets have more tests in there
Oliver Bertuch (Feb 06 2026 at 13:00):Might be an @Parameterized one or single ones.
Oliver Bertuch (Feb 06 2026 at 13:00):Not sure we can make RestAssured hit the other endpoints that easily.
Oliver Bertuch (Feb 06 2026 at 13:19):@Don Sizemore this may be worth another backport to 6.9
Balázs Pataki (Feb 06 2026 at 13:40):I made it ParameterizedTest and now it tests these endpoints
"/api/dataverses/root/datasets",
"/api/v1/dataverses/root/datasets",
"/page_doesnt_exist",
"/dvn/api/data-deposit/v1.1/swordv2/collection/dataverse/root"
Oliver Bertuch (Feb 06 2026 at 14:10):You probably should add CorsIT to the list of tests at https://github.com/IQSS/dataverse/blob/develop/tests/integration-tests.txt
Oliver Bertuch (Feb 06 2026 at 14:10):I've always meant to refactor the API tests to properly use Maven Failsafe and JUnit 5 Suites, but didn't find the time so far...
Oliver Bertuch (Feb 06 2026 at 14:14):I left two comments on the PR
Oliver Bertuch (Feb 06 2026 at 14:28):Edit: added another one.
Oliver Bertuch (Feb 06 2026 at 14:48):@Balázs Pataki I've marked your PR as a draft for now and added all the necessary labels (put didn't put it on the board yet @Philip Durbin 🚀). Keep them changes coming, I'll be the ambassador on including it in 6.10
Philip Durbin 🚀 (Feb 06 2026 at 14:59):I'd say we can safely put it in the "Ready for Triage" column so it comes up on Triage Tuesday. Any objections? Thanks for the PR! :heart:
Philip Durbin 🚀 (Apr 02 2026 at 20:24):@Balázs Pataki @Oliver Bertuch I'm reviewing #12151 and I rebased the branch. I hope you don't mind.
Philip Durbin 🚀 (Apr 02 2026 at 20:25):@Oliver Bertuch I added a note about requirements.txt changes that doc writers will need to know about.
Philip Durbin 🚀 (Apr 02 2026 at 20:26):@Don Sizemore tests are failing on Jenkins. Please see https://github.com/IQSS/dataverse/pull/12151#discussion_r3030000487
Should we enable CORS on the Dataverse instances Jenkins spins up? Can I co-assign you to the PR until we decide how to handle this?
Philip Durbin 🚀 (Apr 02 2026 at 21:06):@Balázs Pataki @Oliver Bertuch I have questions about #12151 and left reviews for you.
Ash Manda (Apr 07 2026 at 19:25):Hello! I am working on Dataverse Ansible to add in the CORS fix to make the CorsIT tests pass.
I was able to allow CORS from DA's side, but I kept running into this issue when testing:
Time elapsed: 0.011 s <<< FAILURE!\norg.opentest4j.AssertionFailedError: Unexpected value for header: Access-Control-Allow-Headers ==> expected: <[range, content-type, x-dataverse-key, accept]> but was: <[*]>\n\
For now, I'm trying to change the Access Control Allow Headers to those exact strings to make sure the tests pass, but I am of the opinion that the assertions are a bit rigid
Philip Durbin 🚀 (Apr 07 2026 at 19:26):@Ash Manda you don't permission to change the test in the PR, right? You'd need the author, @Balázs Pataki, to do that.
Ash Manda (Apr 07 2026 at 19:26):Nope, I can't change it myself.
Philip Durbin 🚀 (Apr 07 2026 at 19:27):Right. Could you please go ahead and leave a review or a comment on the line you're talking about? So @Balázs Pataki can easily find it.
Ash Manda (Apr 07 2026 at 19:30):Done!
Philip Durbin 🚀 (Apr 09 2026 at 12:46):https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-12151/16/testReport/ is passing now! Thanks, @Ash Manda and @Don Sizemore!
Philip Durbin 🚀 (Apr 09 2026 at 12:47):@Balázs Pataki @Oliver Bertuch is CORS on by default now? I'm confused by these docs: https://github.com/IQSS/dataverse/pull/12151#discussion_r3030369558
Ash Manda (Apr 14 2026 at 15:37):@Philip Durbin 🚀 Hi Phil. I see you've approved the CORSfilter PR which is amazing! I just wanted to add that I would still strongly recommend that the assertions for the CorsIT test be rewritten to support "*" and extended headers, before the PR is merged as it makes it very difficult during testing to figure out what exact CORS strings need to be allowed to make the tests pass as giving any additional headers will just fail it.
It should be just 4-5 lines of code change at max, mostly regexes and it makes life easier for everyone involved at the CORS side.
Or you could just merge the PR, and I can open a separate issue later on for a minor fix if you would prefer that.
Thanks!
Philip Durbin 🚀 (Apr 14 2026 at 16:17):@Ash Manda good point! I did see your "request changes" review but I wasn't sure how strongly you feel about it!
@Balázs Pataki or @Oliver Bertuch are you interested in making the changes above?
Ash Manda (Apr 16 2026 at 09:34):I think it might be best to go ahead with the PR merge for now. I'll open a minor fix PR later on for the test failures.
Oliver Bertuch (Apr 16 2026 at 09:37):I had another look today. Currently, the tests simply expect the default values for the Access-Control-Allow-Headers and Access-Control-Expose-Headers when these settings are not configured.
Oliver Bertuch (Apr 16 2026 at 09:38):It sounds like for the Jenkins setup, you're setting them both to "*"
Oliver Bertuch (Apr 16 2026 at 09:38):I understand that y'all want to be cautious with these options, as they may have side effects.
Oliver Bertuch (Apr 16 2026 at 09:39):To test this in a cleaner fashion, we have a few options....
Oliver Bertuch (Apr 16 2026 at 09:47):Environment isolation: deploy a separate test instance just for this test.
- This may be hard in the current Jenkins setup.
- It's easier with containerized tests, but still a lot.
- It would still be overhead but at least easily controlled if we'd create these containers with Testcontainers from within the test.
- Using a separate environment neatly solves the problem that the filter is initialized only once with the values. Changes need a redeploy.
Variation of (1): make it an "integration test", not an API End to End test. We already have some of those using Testcontainers. We can use the packaged container to run Dataverse in it. We'd need some more infrastructure though to setup the Dataverse deployment in it.
Oliver Bertuch (Apr 16 2026 at 09:48):My favorite is (3) as it is the most sustainable effort, not just for CORS tests. It goes well beyond testing, too. The application would have a whole new feature being more "hot reloadable".
Oliver Bertuch (Apr 16 2026 at 09:55):Maybe we should make the test be less opinionated. The test was written to make sure that the CORS headers are present on each of the different endpoints served by different subsystems.
Oliver Bertuch (Apr 16 2026 at 09:55):So instead of asserting the content of the headers, asserting the presence of the headers would be good enough for now.
Oliver Bertuch (Apr 16 2026 at 09:56):We can move the content assertions to a different test that is @Disabled for now until we resolve the tech debt I mentioned above.
Ash Manda (Apr 16 2026 at 09:56):Hi Oliver. I appreciate the detailed reply! From my side, I am being persistent about this because I just want "*" and even lassing additional headers to be considered a valid assertion for the test, pretty much. I am not asking for a rewrite or anything. I dont want to create anya additional load than you already have
Ash Manda (Apr 16 2026 at 09:57):Oliver Bertuch said:
So
Yes, that is exactly what I meant! Thank you! You put it a lot better than I ever could ;)
Oliver Bertuch (Apr 16 2026 at 09:57):I'll change the test now.
Oliver Bertuch (Apr 16 2026 at 10:15):Done.
Oliver Bertuch (Apr 16 2026 at 10:16):I left a whole bunch of Javadocs on the test to explain
Oliver Bertuch (Apr 16 2026 at 10:16):Please let me know how Jenkins is doing :smile:
Ash Manda (Apr 16 2026 at 10:27):Tysm! I'll let you know immediately after the build finishes. Could be an hour or so from now.
Philip Durbin 🚀 (Apr 16 2026 at 12:04):You folks have been busy! @Oliver Bertuch thanks for the commit at https://github.com/IQSS/dataverse/pull/12151/changes/ed07f964cb02bd8cc89e6daa46ff57ecf0fe5c84 . I'm taking a look.
Unfortunately, from here at home, I can't see the details of the Jenkins test run but all the checks in #12151 are green, at least, which is a good sign.
Philip Durbin 🚀 (Apr 16 2026 at 12:12):I think the commit makes sense to me. Just looking for the presence of headers. Seems fine. And future work is explained in a comment. @Ash Manda what do you think?
Ash Manda (Apr 16 2026 at 12:27):@Oliver Bertuch Jenkins looking good, all tests passing
Ash Manda (Apr 16 2026 at 12:28):Philip Durbin 🚀 said:
I think the commit makes sense to me. Just looking for the presence of headers. Seems fine. And future work is explained in a comment. Ash Manda what do you think?
Yep, sounds good to me!
Ash Manda (Apr 16 2026 at 12:28):thank you all for your work on this even though it was supposed to be a small change
Balázs Pataki (Apr 16 2026 at 12:31):Thank you @Oliver Bertuch for solving this!
Oliver Bertuch (Apr 16 2026 at 12:51):Woohoo! Ship it! :ship: :wink:
Oliver Bertuch (Apr 16 2026 at 12:52):No worries @Balázs Pataki. It was a small thing to do once the wrapping around the head was done @Ash Manda :see_no_evil:
Philip Durbin 🚀 (Apr 16 2026 at 13:13):Ash Manda said:
Oliver Bertuch Jenkins looking good, all tests passing
Yes! API tests took 22 minutes: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-12151/18/testReport/
Philip Durbin 🚀 (Apr 16 2026 at 13:15):@Ash Manda do you want to formally remove your "requested changes" status? Alternatively, I can click "dismiss review":
Philip Durbin 🚀 (Apr 16 2026 at 13:16):I did go ahead and move it to "ready for QA" column of the project board.
Ash Manda (Apr 16 2026 at 13:32):Hi Phil, you can go ahead and dismiss it!
Philip Durbin 🚀 (Apr 16 2026 at 14:38):Done! https://github.com/IQSS/dataverse/pull/12151#event-24571051576
Oliver Bertuch (Apr 16 2026 at 14:46): _
.-'` `}
_./) / } SHIP IT! :-)
.'o \ | }
'.___.'`.\ {`
/`\_/ , `. }
\=' .-' _`\ {
`'`;/ `, }
_\ ; }
/__`;-...'--'
Last updated: May 30 2026 at 09:11 UTC