You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by mb...@apache.org on 2021/01/13 19:36:41 UTC
[asterixdb] 02/06: [NO ISSUE][API] Ignore body of GET requests
This is an automated email from the ASF dual-hosted git repository.
mblow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit 1de53b1444a7fcb4871d2549c8625e467c7885be
Author: Ali Alsuliman <al...@gmail.com>
AuthorDate: Fri Jan 8 09:14:11 2021 -0800
[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 <mb...@apache.org>
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
---
.../http/server/QueryServiceRequestParameters.java | 30 ++++++++++++----------
.../api/http/server/QueryServiceServlet.java | 4 +--
.../api/get-non-query/get-non-query.1.get.http | 4 +--
.../api/get-non-query/get-non-query.2.get.http | 5 ++--
.../api/get-non-query/get-non-query.3.get.http | 5 ++--
.../api/get-query/get-query.1.get.http | 4 +--
.../api/get-query/get-query.2.get.http | 5 ++--
.../api/get-query/get-query.3.get.http | 5 ++--
.../ignore-body-for-get.1.get.http} | 8 +++---
.../ignore-body-for-get.2.get.http} | 8 +++---
.../ignore-body-for-get.1.regexadm | 1 +
.../ignore-body-for-get.2.regexadm | 1 +
.../test/resources/runtimets/testsuite_sqlpp.xml | 5 ++++
.../apache/hyracks/http/server/utils/HttpUtil.java | 4 ++-
14 files changed, 55 insertions(+), 34 deletions(-)
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.ObjectMapper;
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 @@ public class QueryServiceRequestParameters {
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 @@ public class QueryServiceRequestParameters {
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 @@ public class QueryServiceServlet extends AbstractQueryApiServlet {
}
@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/get-non-query/get-non-query.3.get.http b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/ignore-body-for-get/ignore-body-for-get.1.get.http
similarity index 78%
copy from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.3.get.http
copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/ignore-body-for-get/ignore-body-for-get.1.get.http
index 4f08b95..ff8b376 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/ignore-body-for-get/ignore-body-for-get.1.get.http
@@ -17,8 +17,10 @@
* under the License.
*/
/*
- * Description: testing passing a non-query statement using GET
- * Result: failure
+ * 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": "CREATE FUNCTION foo(){1};", "readonly": false}
\ No newline at end of file
+--body={"statement": "SELECT 1;"}
\ 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/ignore-body-for-get/ignore-body-for-get.2.get.http
similarity index 76%
copy from asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/get-non-query/get-non-query.3.get.http
copy to asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/ignore-body-for-get/ignore-body-for-get.2.get.http
index 4f08b95..79771fb 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/ignore-body-for-get/ignore-body-for-get.2.get.http
@@ -17,8 +17,10 @@
* under the License.
*/
/*
- * Description: testing passing a non-query statement using GET
- * Result: failure
+ * 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": "CREATE FUNCTION foo(){1};", "readonly": false}
\ No newline at end of file
+--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.DefaultHttpResponse;
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 class HttpUtil {
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) {