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 <[email protected]> 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 <[email protected]> Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> --- .../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) {
