This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new fdfb2e027 [webserver] Restrict UI pages to GET/HEAD methods only
fdfb2e027 is described below
commit fdfb2e027f108849a546e371c0cad4d3323c361e
Author: Gabriella Lotz <[email protected]>
AuthorDate: Mon Nov 10 17:58:56 2025 +0100
[webserver] Restrict UI pages to GET/HEAD methods only
Currently, the web server UI pages return 200 OK regardless of the
HTTP method used. Display pages like "/", "/tables", "/tablet-servers"
accept POST, PUT, DELETE and other methods even though they're purely
informational read-only pages. This could lead to unintended
interactions and doesn't follow HTTP best practices.
This patch adds HTTP method validation for display pages. Pages
registered with StyleMode::STYLED (human-readable UI pages with CSS
and navigation) now only accept GET and HEAD requests. Other HTTP
methods (POST, PUT, DELETE, etc.) return 405 Method Not Allowed with
an appropriate Allow header.
Functional endpoints like /metrics, and REST API endpoints registered
with StyleMode::UNSTYLED or StyleMode::JSON continue to accept all
HTTP methods as before, since they may legitimately need POST for
operations.
The design uses StyleMode as a signal for method restrictions since
STYLED pages are semantically read-only displays for human viewing,
while functional endpoints are machine-consumable and may need to
accept writes. This approach requires no API changes and automatically
applies the correct restrictions based on existing semantics.
Change-Id: Ie232bd50785bb750ecaa0a7e19403e573ac193eb
Reviewed-on: http://gerrit.cloudera.org:8080/23657
Tested-by: Kudu Jenkins
Reviewed-by: Marton Greber <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
src/kudu/server/webserver-test.cc | 101 +++++++++++++++++++++++++++++++++++++-
src/kudu/server/webserver.cc | 33 ++++++++++---
2 files changed, 126 insertions(+), 8 deletions(-)
diff --git a/src/kudu/server/webserver-test.cc
b/src/kudu/server/webserver-test.cc
index 64b0b0a14..be9e7706d 100644
--- a/src/kudu/server/webserver-test.cc
+++ b/src/kudu/server/webserver-test.cc
@@ -186,8 +186,9 @@ class WebserverTest : public KuduTest,
curl_.set_custom_method("OPTIONS");
curl_.set_return_headers(true);
ASSERT_OK(curl_.FetchURL(url_, &buf_));
- ASSERT_STR_CONTAINS(buf_.ToString(),
- "Allow: GET, POST, HEAD, CONNECT, PUT, DELETE,
OPTIONS");
+ // The root "/" is registered as STYLED, so OPTIONS should return
+ // only the allowed methods for STYLED pages.
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Allow: GET, HEAD, OPTIONS");
}
string ServicePrincipalName() const {
@@ -764,6 +765,102 @@ TEST_P(WebserverTest, TestPutMethodNotAllowed) {
ASSERT_EQ("Remote error: HTTP 401", s.ToString());
}
+// Handlers for testing HTTP method restrictions
+static void StyledPageHandler(const Webserver::WebRequest& /*req*/,
+ Webserver::WebResponse* resp) {
+ resp->output["message"] = "Styled page content";
+}
+
+static void FunctionalEndpointHandler(const Webserver::WebRequest& req,
+ Webserver::PrerenderedWebResponse* resp) {
+ resp->output << "Endpoint method: " << req.request_method;
+}
+
+class HttpMethodRestrictionTest : public WebserverTest {
+ protected:
+ void SetUp() override {
+ WebserverTest::SetUp();
+ // Register a styled display page (should be restricted to GET/HEAD)
+ server_->RegisterPathHandler("/styled-page", "Styled", StyledPageHandler,
+ StyleMode::STYLED, true);
+ // Register functional endpoints (should accept all methods)
+ server_->RegisterPrerenderedPathHandler("/functional", "Functional",
+ FunctionalEndpointHandler,
+ StyleMode::UNSTYLED, true);
+ server_->RegisterJsonPathHandler("/json", "JSON",
FunctionalEndpointHandler, false);
+ }
+};
+
+INSTANTIATE_TEST_SUITE_P(Parameters,
+ HttpMethodRestrictionTest,
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
+
+TEST_P(HttpMethodRestrictionTest, TestStyledPageAcceptsGet) {
+ ASSERT_OK(curl_.FetchURL(Substitute("$0/styled-page", url_), &buf_));
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Styled page content");
+}
+
+TEST_P(HttpMethodRestrictionTest, TestStyledPageAcceptsHead) {
+ curl_.set_custom_method("HEAD");
+ ASSERT_OK(curl_.FetchURL(Substitute("$0/styled-page", url_), &buf_));
+}
+
+TEST_P(HttpMethodRestrictionTest, TestStyledPageRejectsPost) {
+ curl_.set_custom_method("POST");
+ Status s = curl_.FetchURL(Substitute("$0/styled-page", url_), &buf_);
+ ASSERT_EQ("Remote error: HTTP 405", s.ToString());
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Method Not Allowed");
+}
+
+TEST_P(HttpMethodRestrictionTest, TestStyledPageRejectsPut) {
+ curl_.set_custom_method("PUT");
+ Status s = curl_.FetchURL(Substitute("$0/styled-page", url_), &buf_);
+ ASSERT_EQ("Remote error: HTTP 405", s.ToString());
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Method Not Allowed");
+}
+
+TEST_P(HttpMethodRestrictionTest, TestStyledPageRejectsDelete) {
+ curl_.set_custom_method("DELETE");
+ Status s = curl_.FetchURL(Substitute("$0/styled-page", url_), &buf_);
+ ASSERT_EQ("Remote error: HTTP 405", s.ToString());
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Method Not Allowed");
+}
+
+TEST_P(HttpMethodRestrictionTest,
TestStyledPageOptionsReturnsCorrectAllowHeader) {
+ // OPTIONS should return only the methods that STYLED pages actually accept
+ curl_.set_custom_method("OPTIONS");
+ curl_.set_return_headers(true);
+ ASSERT_OK(curl_.FetchURL(Substitute("$0/styled-page", url_), &buf_));
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Allow: GET, HEAD, OPTIONS");
+}
+
+TEST_P(HttpMethodRestrictionTest, TestFunctionalEndpointAcceptsPost) {
+ // Verify functional endpoints (UNSTYLED) accept POST even if on nav bar
+ ASSERT_OK(curl_.PostToURL(Substitute("$0/functional", url_), "test", &buf_));
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Endpoint method: POST");
+}
+
+TEST_P(HttpMethodRestrictionTest, TestJSONEndpointAcceptsPost) {
+ ASSERT_OK(curl_.PostToURL(Substitute("$0/json", url_), "test", &buf_));
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Endpoint method: POST");
+}
+
+TEST_P(HttpMethodRestrictionTest,
TestFunctionalEndpointOptionsReturnsAllMethods) {
+ // Verify OPTIONS on UNSTYLED endpoints returns full set of allowed methods
+ curl_.set_custom_method("OPTIONS");
+ curl_.set_return_headers(true);
+ ASSERT_OK(curl_.FetchURL(Substitute("$0/functional", url_), &buf_));
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Allow: GET, POST, HEAD, PUT, DELETE,
OPTIONS");
+}
+
+TEST_P(HttpMethodRestrictionTest, TestJSONEndpointOptionsReturnsAllMethods) {
+ // Verify OPTIONS on JSON endpoints returns full set of allowed methods
+ curl_.set_custom_method("OPTIONS");
+ curl_.set_return_headers(true);
+ ASSERT_OK(curl_.FetchURL(Substitute("$0/json", url_), &buf_));
+ ASSERT_STR_CONTAINS(buf_.ToString(), "Allow: GET, POST, HEAD, PUT, DELETE,
OPTIONS");
+}
+
// Test that authenticated principal is correctly passed to the handler.
static void Handler(const Webserver::WebRequest& req,
Webserver::PrerenderedWebResponse* resp) {
resp->output << req.username;
diff --git a/src/kudu/server/webserver.cc b/src/kudu/server/webserver.cc
index c39d3283a..90824ef7f 100644
--- a/src/kudu/server/webserver.cc
+++ b/src/kudu/server/webserver.cc
@@ -591,16 +591,14 @@ sq_callback_result_t
Webserver::BeginRequestCallbackStatic(
sq_callback_result_t Webserver::BeginRequestCallback(
struct sq_connection* connection,
struct sq_request_info* request_info) {
- if (strncmp("OPTIONS", request_info->request_method, 7) == 0) {
- // Let Squeasel deal with the request. OPTIONS requests should not require
- // authentication, so do this before doing SPNEGO.
- return SQ_CONTINUE_HANDLING;
- }
+ // OPTIONS requests should not require authentication, but we need to handle
+ // them in RunPathHandler to return the correct Allow header for STYLED
pages.
+ bool is_options = strncmp("OPTIONS", request_info->request_method, 7) == 0;
// The last SPNEGO step in a successful authentication may include a response
// header (e.g. when using mutual authentication).
PrerenderedWebResponse resp;
- if (opts_.require_spnego) {
+ if (opts_.require_spnego && !is_options) {
const char* authz_header = sq_get_header(connection, "Authorization");
string authn_princ;
Status s = RunSpnegoStep(authz_header, &resp.response_headers,
&authn_princ);
@@ -742,6 +740,29 @@ sq_callback_result_t Webserver::RunPathHandler(
req.request_headers[key] = h.value;
}
req.request_method = request_info->request_method;
+
+ if (req.request_method == "OPTIONS") {
+ resp->status_code = HttpStatusCode::Ok;
+ if (handler.style_mode() == StyleMode::STYLED) {
+ resp->response_headers["Allow"] = "GET, HEAD, OPTIONS";
+ } else {
+ resp->response_headers["Allow"] = "GET, POST, HEAD, PUT, DELETE,
OPTIONS";
+ }
+ SendResponse(connection, resp);
+ return SQ_HANDLED_OK;
+ }
+
+ // Restrict display pages (StyleMode::STYLED) to GET/HEAD methods only.
+ // Functional endpoints (pprof, metrics, etc.) legitimately need POST/PUT.
+ if (handler.style_mode() == StyleMode::STYLED &&
+ req.request_method != "GET" && req.request_method != "HEAD") {
+ resp->status_code = HttpStatusCode::MethodNotAllowed;
+ resp->response_headers["Allow"] = "GET, HEAD";
+ resp->output << "Method Not Allowed";
+ SendResponse(connection, resp);
+ return SQ_HANDLED_OK;
+ }
+
if (req.request_method == "POST" || req.request_method == "PUT") {
const char* content_len_str = sq_get_header(connection, "Content-Length");
int32_t content_len = 0;
