You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by tm...@apache.org on 2019/10/01 23:43:12 UTC
[impala] branch master updated: IMPALA-8982: Fix webserver handling
of OPTIONS
This is an automated email from the ASF dual-hosted git repository.
tmarshall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new eb6116d IMPALA-8982: Fix webserver handling of OPTIONS
eb6116d is described below
commit eb6116d49df8817afaa5bd0b12cd32894901cfa9
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
AuthorDate: Tue Sep 24 12:43:37 2019 -0700
IMPALA-8982: Fix webserver handling of OPTIONS
Previously, Impala's webserver always returned 500 errors to OPTIONS
requests. Fixing this requires two changes:
- Squeasel is updated to 36dc66b8723980feb32196a94b46734eb5eafbf9,
which pulls in a patch that allows it to handle OPTIONS requests
correctly.
- Webserver is modified to recognize OPTIONS requests and allow
Squeasel to handle them.
Testing:
- Added a BE test that checks that OPTIONS works without SPNEGO.
- Added a test that checks that OPTIONS works when SPNEGO is turned
on. The test is currently disabled until we add curl to the
toolchain.
Change-Id: Ie4eb88c34348a7204686fae0d61a6d5546ef3a34
Reviewed-on: http://gerrit.cloudera.org:8080/14323
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/thirdparty/squeasel/squeasel.c | 11 +++++++++--
be/src/util/webserver-test.cc | 20 +++++++++++++++++---
be/src/util/webserver.cc | 6 ++++++
3 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/be/src/thirdparty/squeasel/squeasel.c b/be/src/thirdparty/squeasel/squeasel.c
index b301f5c..16c8ff4 100644
--- a/be/src/thirdparty/squeasel/squeasel.c
+++ b/be/src/thirdparty/squeasel/squeasel.c
@@ -209,7 +209,7 @@ struct socket {
};
char* SAFE_HTTP_METHODS[] = {
- "GET", "POST", "HEAD", "PROPFIND", "MKCOL" };
+ "GET", "POST", "HEAD", "PROPFIND", "MKCOL", "OPTIONS" };
// See https://www.owasp.org/index.php/Test_HTTP_Methods_(OTG-CONFIG-006) for details.
#ifdef ALLOW_UNSAFE_HTTP_METHODS
@@ -3246,9 +3246,16 @@ static void handle_ssi_file_request(struct sq_connection *conn,
static void send_options(struct sq_connection *conn) {
conn->status_code = 200;
+ #ifdef ALLOW_UNSAFE_HTTP_METHODS
sq_printf(conn, "%s", "HTTP/1.1 200 OK\r\n"
"Allow: GET, POST, HEAD, CONNECT, PUT, DELETE, OPTIONS, PROPFIND, MKCOL\r\n"
- "DAV: 1\r\n\r\n");
+ "DAV: 1\r\n"
+ "Content-Length: 0\r\n\r\n");
+ #else
+ sq_printf(conn, "%s", "HTTP/1.1 200 OK\r\n"
+ "Allow: GET, POST, HEAD, OPTIONS, PROPFIND, MKCOL\r\n"
+ "Content-Length: 0\r\n\r\n");
+ #endif
}
// Writes PROPFIND properties for a collection element
diff --git a/be/src/util/webserver-test.cc b/be/src/util/webserver-test.cc
index 74527f8..0ea9a41 100644
--- a/be/src/util/webserver-test.cc
+++ b/be/src/util/webserver-test.cc
@@ -59,13 +59,13 @@ const string ESCAPED_VALUE = "<script language='javascript'>";
// Adapted from:
// http://stackoverflow.com/questions/10982717/get-html-without-header-with-boostasio
Status HttpGet(const string& host, const int32_t& port, const string& url_path,
- ostream* out, int expected_code = 200) {
+ ostream* out, int expected_code = 200, const string& method = "GET") {
try {
tcp::iostream request_stream;
request_stream.connect(host, lexical_cast<string>(port));
if (!request_stream) return Status("Could not connect request_stream");
- request_stream << "GET " << url_path << " HTTP/1.1\r\n";
+ request_stream << method << " " << url_path << " HTTP/1.1\r\n";
request_stream << "Host: " << host << ":" << port << "\r\n";
request_stream << "Accept: */*\r\n";
request_stream << "Cache-Control: no-cache\r\n";
@@ -348,8 +348,14 @@ TEST(Webserver, TestWithSpnego) {
stringstream contents;
ASSERT_FALSE(HttpGet("localhost", FLAGS_webserver_port, "/", &contents).ok());
- // TODO(todd): import curl into native-toolchain and test this with
+ // TODO(todd) IMPALA-8987: import curl into native-toolchain and test this with
// authentication.
+ // Test that OPTIONS works with and without having kinit-ed.
+ //string options_cmd = Substitute(
+ // "curl -X OPTIONS -v --negotiate -u : 'http://127.0.0.1:$0'", FLAGS_webserver_port);
+ //system(options_cmd.c_str());
+ //KUDU_ASSERT_OK(kdc.Kinit("alice"));
+ //system(options_cmd.c_str());
}
TEST(Webserver, StartWithPasswordFileTest) {
@@ -438,6 +444,14 @@ TEST(Webserver, NullCharTest) {
ASSERT_TRUE(contents.str().find(STRING_WITH_NULL) != string::npos);
}
+TEST(Webserver, Options) {
+ Webserver webserver("", FLAGS_webserver_port);
+ ASSERT_OK(webserver.Start());
+ stringstream contents;
+ ASSERT_OK(HttpGet("localhost", FLAGS_webserver_port, "/", &contents, 200, "OPTIONS"));
+ ASSERT_FALSE(contents.str().find("Allow: GET, POST, HEAD, OPTIONS, PROPFIND, MKCOL")
+ == string::npos);
+}
int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
diff --git a/be/src/util/webserver.cc b/be/src/util/webserver.cc
index 8ffc871..d43fb6d 100644
--- a/be/src/util/webserver.cc
+++ b/be/src/util/webserver.cc
@@ -490,6 +490,12 @@ 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;
+ }
+
if (FLAGS_webserver_require_spnego){
sq_callback_result_t spnego_result = HandleSpnego(connection, request_info);
if (spnego_result != SQ_CONTINUE_HANDLING) {