[MINOR] Tighten origin and content-type handling in REST/WebSocket layer#5229
Open
jongyoul wants to merge 2 commits intoapache:masterfrom
Open
[MINOR] Tighten origin and content-type handling in REST/WebSocket layer#5229jongyoul wants to merge 2 commits intoapache:masterfrom
jongyoul wants to merge 2 commits intoapache:masterfrom
Conversation
Apply stricter defaults to the request-handling layer: - CorsFilter blocks state-changing methods (POST/PUT/DELETE/PATCH) and cross-origin preflight requests when the Origin header is not in the configured allow-list. Access-Control-Allow-Credentials is only sent when the Origin is allowed. - The default value of zeppelin.server.allowed.origins changes from "*" to empty so cross-origin browser access must be explicitly enabled. Operators relying on the previous default need to set this back to "*" or to specific origin(s). Same-origin/same-host and non-browser clients are unaffected. - A new request filter restricts REST request bodies on state-changing methods to application/json, application/x-www-form-urlencoded, or multipart/form-data; other media types are rejected with 415. - The default shiro.ini.template now sets cookie.sameSite = LAX. - ZeppelinClient.addParagraph and updateParagraph now send an explicit Content-Type: application/json header so they pass the new filter. - CorsUtils.isValidOrigin normalizes the Origin header to lowercase before the allow-list membership check, mirroring how the configured origins are stored, so case differences in the Origin header do not produce false rejections. - A small HttpMethods utility holds the shared STATE_CHANGING method set used by both the servlet filter and the Jersey filter.
There was a problem hiding this comment.
Pull request overview
This PR tightens default security behavior in Zeppelin’s REST/WebSocket request-handling layer by restricting cross-origin state-changing requests and enforcing an allow-list of request body media types.
Changes:
- Harden CORS behavior: block disallowed cross-origin preflights and state-changing methods; only emit
Access-Control-Allow-Credentialsfor allowed origins; default allowed origins becomes empty. - Add a Jersey request filter to reject unsupported
Content-Typeon state-changing REST requests with bodies (415). - Update client and config templates to align with stricter defaults (explicit JSON
Content-Typein some client calls; Shiro cookie SameSite default).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| zeppelin-server/src/test/java/org/apache/zeppelin/server/CorsFilterTest.java | Expands CORS filter tests for blocked/allowed origin and method behaviors. |
| zeppelin-server/src/test/java/org/apache/zeppelin/rest/filter/JsonContentTypeFilterTest.java | Adds unit tests validating the new REST content-type allow-list behavior. |
| zeppelin-server/src/test/java/org/apache/zeppelin/conf/ZeppelinConfigurationTest.java | Updates allowed-origins default expectation to “empty list”. |
| zeppelin-server/src/main/java/org/apache/zeppelin/utils/HttpMethods.java | Introduces shared state-changing HTTP method set. |
| zeppelin-server/src/main/java/org/apache/zeppelin/utils/CorsUtils.java | Normalizes Origin casing before allow-list membership checks. |
| zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java | Registers the new Jersey request filter. |
| zeppelin-server/src/main/java/org/apache/zeppelin/server/CorsFilter.java | Tightens CORS handling (blocking disallowed cross-origin preflight/state-changing requests; conditional credentials header). |
| zeppelin-server/src/main/java/org/apache/zeppelin/rest/filter/JsonContentTypeFilter.java | Adds filter restricting request body media types for state-changing methods. |
| zeppelin-server/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java | Changes default zeppelin.server.allowed.origins from * to empty. |
| zeppelin-client/src/main/java/org/apache/zeppelin/client/ZeppelinClient.java | Adds explicit JSON Content-Type for two paragraph APIs. |
| conf/shiro.ini.template | Sets default cookie.sameSite = LAX in the template. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+43
to
+53
| String currentHost = InetAddress.getLocalHost().getHostName().toLowerCase(Locale.ROOT); | ||
| // getAllowedOrigins() returns lowercased entries; normalize sourceHost the same way | ||
| // before the membership check so case differences in the Origin header do not produce | ||
| // false rejections of explicitly configured origins. | ||
| String normalizedOrigin = | ||
| sourceHost == null ? "" : sourceHost.toLowerCase(Locale.ROOT); | ||
|
|
||
| return zConf.getAllowedOrigins().contains("*") | ||
| || currentHost.equals(sourceUriHost) | ||
| || "localhost".equals(sourceUriHost) | ||
| || zConf.getAllowedOrigins().contains(sourceHost); | ||
| || zConf.getAllowedOrigins().contains(normalizedOrigin); |
Comment on lines
+58
to
+70
| if (sourceHost != null && !sourceHost.isEmpty()) { | ||
| try { | ||
| if (CorsUtils.isValidOrigin(sourceHost, zConf)) { | ||
| allowedOrigin = sourceHost; | ||
| } | ||
| } catch (URISyntaxException e) { | ||
| LOGGER.warn("Rejecting request with malformed Origin header: {}", sourceHost); | ||
| } | ||
| } catch (URISyntaxException e) { | ||
| LOGGER.error("Exception in WebDriverManager while getWebDriver ", e); | ||
| } | ||
|
|
||
| if (((HttpServletRequest) request).getMethod().equals("OPTIONS")) { | ||
| HttpServletResponse resp = ((HttpServletResponse) response); | ||
| addCorsHeaders(resp, origin); | ||
| return; | ||
| if (allowedOrigin.isEmpty() && (isCorsPreflight(httpRequest) || isStateChanging(method))) { | ||
| LOGGER.warn("Blocking cross-origin {} request from disallowed Origin: {}", | ||
| method, sourceHost); | ||
| httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, "Origin not allowed"); |
Comment on lines
593
to
597
| HttpResponse<JsonNode> response = Unirest.post("/notebook/{noteId}/paragraph") | ||
| .routeParam("noteId", noteId) | ||
| .header("Content-Type", "application/json") | ||
| .body(bodyObject.toString()) | ||
| .asJson(); |
Comment on lines
+30
to
+42
| /** | ||
| * Restricts the request body media types accepted by REST endpoints to a small allow-list. | ||
| * Requests carrying state-changing methods (POST/PUT/DELETE/PATCH) with a body must use | ||
| * {@code application/json}, {@code application/x-www-form-urlencoded}, or | ||
| * {@code multipart/form-data}; anything else is rejected with 415. | ||
| */ | ||
| @Provider | ||
| public class JsonContentTypeFilter implements ContainerRequestFilter { | ||
|
|
||
| private static final Set<String> ALLOWED_TYPES = Set.of( | ||
| "application/json", | ||
| "application/x-www-form-urlencoded", | ||
| "multipart/form-data"); |
ZeppelinClient state-changing calls and the AbstractTestRestApi PUT helper now declare application/json so the new content-type filter accepts them. Aligns the rest of the surface with addParagraph and updateParagraph, which already set the header.
tbonelee
approved these changes
May 9, 2026
Contributor
tbonelee
left a comment
tbonelee
left a comment
There was a problem hiding this comment.
LGTM, approving. A couple of small suggestions you might consider:
- Could we rename JsonContentTypeFilter? It also accepts form-urlencoded and multipart, so something like MediaTypeAllowlistFilter might describe it better. Probably easier to do before merge.
- The new allowed.origins default and Content-Type rejection will break some external clients on upgrade. It'd be great to have these called out in the next release notes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this PR for?
Apply stricter defaults to the request-handling layer for tighter out-of-the-box behavior:
CorsFilterblocks state-changing methods (POST/PUT/DELETE/PATCH) and cross-origin preflight requests when theOriginheader is not in the configured allow-list.Access-Control-Allow-Credentialsis only sent when theOriginis allowed.zeppelin.server.allowed.originschanges from*to empty so cross-origin browser access must be explicitly enabled. Operators relying on the previous default need to set this back to*or to specific origin(s). Same-origin / same-host and non-browser clients are unaffected.application/json,application/x-www-form-urlencoded, ormultipart/form-data; other media types are rejected with415.shiro.ini.templatenow setscookie.sameSite = LAX.ZeppelinClient.addParagraphandupdateParagraphnow send an explicitContent-Type: application/jsonheader so they pass the new filter.CorsUtils.isValidOriginnormalizes theOriginheader to lowercase before the allow-list membership check, mirroring how the configured origins are stored, so case differences in theOriginheader do not produce false rejections.HttpMethodsutility holds the sharedSTATE_CHANGINGmethod set used by both the servlet filter and the Jersey filter.What type of PR is it?
Improvement
Todos
Questions
Screenshots (if appropriate)
N/A