[NO ISSUE][API] Ignore body of GET requests
- user model changes: yes
- storage format changes: no
- interface changes: no
Change-Id: Ia2db86bab403ef881880388775c00b497955dde8
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/9483
Reviewed-by: Michael Blow <mblow@apache.org>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceRequestParameters.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceRequestParameters.java
index 2379e69..06f8325 100644
--- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceRequestParameters.java
+++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceRequestParameters.java
@@ -51,6 +51,8 @@
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.collect.ImmutableMap;
+import io.netty.handler.codec.http.HttpMethod;
+
public class QueryServiceRequestParameters {
public enum Parameter {
@@ -375,32 +377,36 @@
throws IOException {
setHost(servlet.host(request));
setPath(servlet.servletPath(request));
- String contentType = HttpUtil.getContentTypeOnly(request);
setOptionalParams(optionalParams);
try {
- if (HttpUtil.ContentType.APPLICATION_JSON.equals(contentType)) {
- setParamFromJSON(request, optionalParams);
+ if (useRequestParameters(request)) {
+ setFromRequestParameters(request);
} else {
- setParamFromRequest(request, optionalParams);
+ setFromRequestBody(request);
}
} catch (JsonParseException | JsonMappingException e) {
throw new RuntimeDataException(ErrorCode.INVALID_REQ_JSON_VAL);
}
}
- private void setParamFromJSON(IServletRequest request, Map<String, String> optionalParameters) throws IOException {
+ private boolean useRequestParameters(IServletRequest request) {
+ String contentType = HttpUtil.getContentTypeOnly(request);
+ HttpMethod method = request.getHttpRequest().method();
+ return HttpMethod.GET.equals(method) || !HttpUtil.ContentType.APPLICATION_JSON.equals(contentType);
+ }
+
+ private void setFromRequestBody(IServletRequest request) throws IOException {
JsonNode jsonRequest = OBJECT_MAPPER.readTree(HttpUtil.getRequestBody(request));
setParams(jsonRequest, request.getHeader(HttpHeaders.ACCEPT), QueryServiceRequestParameters::getParameter);
setStatementParams(getOptStatementParameters(jsonRequest, jsonRequest.fieldNames(), JsonNode::get, v -> v));
- setJsonOptionalParameters(jsonRequest, optionalParameters);
+ setExtraParams(jsonRequest);
}
- private void setParamFromRequest(IServletRequest request, Map<String, String> optionalParameters)
- throws IOException {
+ private void setFromRequestParameters(IServletRequest request) throws IOException {
setParams(request, request.getHeader(HttpHeaders.ACCEPT), IServletRequest::getParameter);
setStatementParams(getOptStatementParameters(request, request.getParameterNames().iterator(),
IServletRequest::getParameter, OBJECT_MAPPER::readTree));
- setOptionalParameters(request, optionalParameters);
+ setExtraParams(request);
}
private <Req> void setParams(Req req, String acceptHeader, BiFunction<Req, String, String> valGetter)
@@ -431,13 +437,11 @@
setSignature(parseBoolean(req, Parameter.SIGNATURE.str(), valGetter, isSignature()));
}
- protected void setJsonOptionalParameters(JsonNode jsonRequest, Map<String, String> optionalParameters)
- throws HyracksDataException {
+ protected void setExtraParams(JsonNode jsonRequest) throws HyracksDataException {
// allows extensions to set extra parameters
}
- protected void setOptionalParameters(IServletRequest request, Map<String, String> optionalParameters)
- throws HyracksDataException {
+ protected void setExtraParams(IServletRequest request) throws HyracksDataException {
// allows extensions to set extra parameters
}
diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java
index 440b351..254bdae 100644
--- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java
+++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java
@@ -135,12 +135,12 @@
}
@Override
- protected void get(IServletRequest request, IServletResponse response) throws IOException {
+ protected final void get(IServletRequest request, IServletResponse response) throws IOException {
handleRequest(request, response, true);
}
@Override
- protected void post(IServletRequest request, IServletResponse response) throws IOException {
+ protected final void post(IServletRequest request, IServletResponse response) throws IOException {
handleRequest(request, response, false);
}
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.1.get.http b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.1.get.http
index e6aff85..bbacf58 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.1.get.http
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.1.get.http
@@ -20,5 +20,5 @@
* Description: testing passing a non-query statement using GET
* Result: failure
*/
-/query/service
---body={"statement": "CREATE DATAVERSE dv1;"}
\ No newline at end of file
+# param statement=CREATE DATAVERSE dv1
+/query/service
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.2.get.http b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.2.get.http
index 5dc6ec7..71153c9 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.2.get.http
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.2.get.http
@@ -20,5 +20,6 @@
* Description: testing passing a non-query statement using GET
* Result: failure
*/
-/query/service
---body={"statement": "CREATE TYPE t1 AS {id: int};", "readonly": true}
\ No newline at end of file
+# param statement=CREATE TYPE t1 AS {id: int}
+# param readonly=true
+/query/service
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.3.get.http b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.3.get.http
index 4f08b95..ba3c68e 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.3.get.http
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.3.get.http
@@ -20,5 +20,6 @@
* Description: testing passing a non-query statement using GET
* Result: failure
*/
-/query/service
---body={"statement": "CREATE FUNCTION foo(){1};", "readonly": false}
\ No newline at end of file
+# param statement=CREATE FUNCTION foo(){1}
+# param readonly=false
+/query/service
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.1.get.http b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.1.get.http
index 4743d9f..f17053a 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.1.get.http
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.1.get.http
@@ -21,5 +21,5 @@
* Result: success
*/
-- extractresult=true
-/query/service
---body={"statement": "FROM Metadata.`Dataverse` v WHERE v.DataverseName = 'Metadata' SELECT VALUE v.DataverseName;"}
\ No newline at end of file
+# param statement=FROM Metadata.`Dataverse` v WHERE v.DataverseName = 'Metadata' SELECT VALUE v.DataverseName
+/query/service
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.2.get.http b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.2.get.http
index 8995d4f..57dc1ca 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.2.get.http
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.2.get.http
@@ -21,5 +21,6 @@
* Result: success
*/
-- extractresult=true
-/query/service
---body={"statement": "FROM Metadata.`Dataverse` v WHERE v.DataverseName = 'Metadata' SELECT VALUE v.DataverseName;", "readonly": false}
\ No newline at end of file
+# param statement=FROM Metadata.`Dataverse` v WHERE v.DataverseName = 'Metadata' SELECT VALUE v.DataverseName
+# param readonly=false
+/query/service
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.3.get.http b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.3.get.http
index f821428..14c90a0 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.3.get.http
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-query/get-query.3.get.http
@@ -21,5 +21,6 @@
* Result: success
*/
-- extractresult=true
-/query/service
---body={"statement": "FROM Metadata.`Dataverse` v WHERE v.DataverseName = 'Metadata' SELECT VALUE v.DataverseName;", "readonly": true}
\ No newline at end of file
+# param statement=FROM Metadata.`Dataverse` v WHERE v.DataverseName = 'Metadata' SELECT VALUE v.DataverseName
+# param readonly=true
+/query/service
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/ignore-body-for-get/ignore-body-for-get.1.get.http b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/ignore-body-for-get/ignore-body-for-get.1.get.http
new file mode 100644
index 0000000..ff8b376
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/ignore-body-for-get/ignore-body-for-get.1.get.http
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/*
+ * Description: testing that body of GET requests are ignored
+ * Result: failure (missing the required "statement" parameter since it will be ignored)
+ */
+// statuscode 400
+-- requesttype=application/json
+/query/service
+--body={"statement": "SELECT 1;"}
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/ignore-body-for-get/ignore-body-for-get.2.get.http b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/ignore-body-for-get/ignore-body-for-get.2.get.http
new file mode 100644
index 0000000..79771fb
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/ignore-body-for-get/ignore-body-for-get.2.get.http
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/*
+ * Description: testing that body of GET requests are ignored
+ * Result: failure (missing the required "statement" parameter since it will be ignored)
+ */
+// statuscode 400
+-- requesttype=application/x-www-form-urlencoded
+/query/service
+--body={"statement": "SELECT 1;"}
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/api/ignore-body-for-get/ignore-body-for-get.1.regexadm b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/ignore-body-for-get/ignore-body-for-get.1.regexadm
new file mode 100644
index 0000000..73efe45
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/ignore-body-for-get/ignore-body-for-get.1.regexadm
@@ -0,0 +1 @@
+.*No statement provided.*
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/api/ignore-body-for-get/ignore-body-for-get.2.regexadm b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/ignore-body-for-get/ignore-body-for-get.2.regexadm
new file mode 100644
index 0000000..73efe45
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/ignore-body-for-get/ignore-body-for-get.2.regexadm
@@ -0,0 +1 @@
+.*No statement provided.*
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
index ff37ac5..c8e5bfa 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -127,6 +127,11 @@
<expected-error>TYPE_DECL statement is not supported in read-only mode</expected-error>
</compilation-unit>
</test-case>
+ <test-case FilePath="api">
+ <compilation-unit name="ignore-body-for-get">
+ <output-dir compare="Text">ignore-body-for-get</output-dir>
+ </compilation-unit>
+ </test-case>
</test-group>
<test-group name="flwor">
<test-case FilePath="flwor">
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java
index f9ada92..1f0d3bc 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java
@@ -53,6 +53,7 @@
import io.netty.handler.codec.http.FullHttpRequest;
import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpHeaderValues;
+import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpRequest;
import io.netty.handler.codec.http.HttpScheme;
import io.netty.util.AsciiString;
@@ -76,7 +77,8 @@
public static IServletRequest toServletRequest(ChannelHandlerContext ctx, FullHttpRequest request,
HttpScheme scheme) {
return ContentType.APPLICATION_X_WWW_FORM_URLENCODED.equals(getContentTypeOnly(request))
- ? FormUrlEncodedRequest.create(ctx, request, scheme) : BaseRequest.create(ctx, request, scheme);
+ && !HttpMethod.GET.equals(request.method()) ? FormUrlEncodedRequest.create(ctx, request, scheme)
+ : BaseRequest.create(ctx, request, scheme);
}
public static String getContentTypeOnly(IServletRequest request) {