Skip to content

fix(hubble): cache CORS preflight responses - #750

Open
cragkhit wants to merge 1 commit into
apache:masterfrom
cragkhit:matcha-cache-hubble-cors-preflight
Open

fix(hubble): cache CORS preflight responses#750
cragkhit wants to merge 1 commit into
apache:masterfrom
cragkhit:matcha-cache-hubble-cors-preflight

Conversation

@cragkhit

@cragkhit cragkhit commented Aug 1, 2026

Copy link
Copy Markdown

Hi. We are researchers from Mahidol University, Thailand, and the State University of Ceará, Brazil, working on a research project for improving open-source projects by using the latest accepted answer from Stack Overflow that matched your code snippet. We found this recommendation for improving your code from https://stackoverflow.com/a/58370217.

Note: Our study is approved by the Institutional Review Board of Mahidol University. You can find the participant information sheet explaining this study https://drive.google.com/file/d/1ml5AqrtWQ9pnifTQyTFTcWQmwp6RuPA7/view?usp=sharing.


Proposed change

Added a one-hour preflight cache duration to Hubble's global CORS configuration.

Added a one-hour preflight cache duration to Hubble's global CORS configuration.
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. hubble-be labels Aug 1, 2026
@github-actions github-actions Bot added the hubble hugegraph-hubble label Aug 1, 2026

@imbajin imbajin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking: yes. Summary: The configured preflight cache is unreachable under the current same-origin-only policy, so this change cannot produce the advertised response header. Evidence: Spring Web 5.1.9 rejects an empty allowedOrigins configuration before writing maxAge; the Hubble backend build, Checkstyle, and 394 unit tests pass.

config.setAllowCredentials(false);
config.addAllowedMethod("*");
config.addAllowedHeader("*");
config.setMaxAge(3600L);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

‼️ Critical: the configured preflight cache is unreachable. This CorsConfiguration never sets allowedOrigins; in the pinned Spring Web 5.1.9, checkOrigin() returns null for an empty origin list and DefaultCorsProcessor rejects the preflight before it reaches the branch that writes Access-Control-Max-Age. Same-origin requests do not need CORS preflight processing, so this line cannot implement the PR's stated behavior. Please either remove this ineffective setting while Hubble remains same-origin-only, or first add an explicit trusted-origin configuration and an OPTIONS regression test proving that an allowed origin receives Access-Control-Max-Age: 3600 while untrusted origins remain rejected.

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 32.91%. Comparing base (b066b80) to head (099f745).
⚠️ Report is 196 commits behind head on master.

Files with missing lines Patch % Lines
.../org/apache/hugegraph/config/GlobalCorsConfig.java 0.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (b066b80) and HEAD (099f745). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (b066b80) HEAD (099f745)
2 1
Additional details and impacted files
@@              Coverage Diff              @@
##             master     #750       +/-   ##
=============================================
- Coverage     62.49%   32.91%   -29.58%     
+ Complexity     1903     1572      -331     
=============================================
  Files           262      264        +2     
  Lines          9541    14311     +4770     
  Branches        886     1743      +857     
=============================================
- Hits           5963     4711     -1252     
- Misses         3190     9068     +5878     
- Partials        388      532      +144     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hubble hugegraph-hubble hubble-be size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants