You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by al...@apache.org on 2020/11/20 20:27:47 UTC
[asterixdb] branch master updated: [ASTERIXDB-2805][API] Fix http
server handling of duplicate parameters
This is an automated email from the ASF dual-hosted git repository.
alsuliman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git
The following commit(s) were added to refs/heads/master by this push:
new 7cd610e [ASTERIXDB-2805][API] Fix http server handling of duplicate parameters
7cd610e is described below
commit 7cd610ebbb02958f17d423da6e28a36d5721e2fe
Author: Ali Alsuliman <al...@gmail.com>
AuthorDate: Fri Nov 20 09:05:36 2020 -0800
[ASTERIXDB-2805][API] Fix http server handling of duplicate parameters
- user model changes: yes
- storage format changes: no
- interface changes: yes
Details:
Do not reduce multiple parameter values into one value
(i.e. joining the values using a comma). Keep multiple
values separate. For APIs that accept a single value,
return the first value.
Change-Id: I115caa4b2ad890cb5fc30cc3c67514b5cba7049a
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8946
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Michael Blow <mb...@apache.org>
---
.../asterix/api/http/server/ServletUtil.java | 2 +-
.../multiple-param-values.1.get.http | 23 ++++++++++++++++
.../multiple-param-values.1.regexjson | 12 ++++++++
.../test/resources/runtimets/testsuite_sqlpp.xml | 7 ++++-
.../apache/hyracks/http/api/IServletRequest.java | 23 ++++++++++------
.../apache/hyracks/http/server/BaseRequest.java | 32 ++++++++++++++--------
.../apache/hyracks/http/server/utils/HttpUtil.java | 6 ----
7 files changed, 77 insertions(+), 28 deletions(-)
diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ServletUtil.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ServletUtil.java
index 9e85c82..b97d88a 100644
--- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ServletUtil.java
+++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ServletUtil.java
@@ -52,6 +52,6 @@ public class ServletUtil {
public static DataverseName getDataverseName(IServletRequest request, String dataverseParameterName) {
List<String> values = request.getParameterValues(dataverseParameterName);
- return values != null ? DataverseName.create(values) : null;
+ return !values.isEmpty() ? DataverseName.create(values) : null;
}
}
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/multiple-param-values/multiple-param-values.1.get.http b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/multiple-param-values/multiple-param-values.1.get.http
new file mode 100644
index 0000000..5e92649
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/multiple-param-values/multiple-param-values.1.get.http
@@ -0,0 +1,23 @@
+/*
+ * 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 passing multiple values for a parameter
+ * Result: success (the first value of a parameter is chosen)
+ */
+/query/service?statement=SELECT%201;&statement=SELECT%202&optimized-logical-plan=false&optimized-logical-plan=true;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/api/multiple-param-values/multiple-param-values.1.regexjson b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/multiple-param-values/multiple-param-values.1.regexjson
new file mode 100644
index 0000000..0701ef3
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/api/multiple-param-values/multiple-param-values.1.regexjson
@@ -0,0 +1,12 @@
+{
+ "requestID": "R{.*}",
+ "signature": {
+ "*": "*"
+ },
+ "type": "application/x-adm",
+ "results": [ "{ \"$1\": 1 }\n" ]
+ ,
+ "plans":{},
+ "status": "success",
+ "metrics": "R{.*}"
+}
\ 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 be711d3..df8de8d 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -27,7 +27,12 @@
&AsyncDeferredQueries;
&GeoQueries;
&TemporalQueries;
- <test-group name="flwor">
+ <test-group name="api">
+ <test-case FilePath="api">
+ <compilation-unit name="multiple-param-values">
+ <output-dir compare="Text">multiple-param-values</output-dir>
+ </compilation-unit>
+ </test-case>
<test-case FilePath="api">
<compilation-unit name="readonly-request">
<output-dir compare="Text">readonly-request</output-dir>
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServletRequest.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServletRequest.java
index a8996e3..a049412 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServletRequest.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServletRequest.java
@@ -36,9 +36,9 @@ public interface IServletRequest {
FullHttpRequest getHttpRequest();
/**
- * Get a request parameter
+ * Get a request parameter. If there are multiple values, it returns the first one.
*
- * @param name
+ * @param name parameter name
* @return the parameter or null if not found
*/
String getParameter(CharSequence name);
@@ -46,8 +46,8 @@ public interface IServletRequest {
/**
* Get all values of a request parameter
*
- * @param name
- * @return the parameter values or null if not found
+ * @param name parameter name
+ * @return the parameter values or empty list if not found
*/
List<String> getParameterValues(CharSequence name);
@@ -59,16 +59,23 @@ public interface IServletRequest {
Set<String> getParameterNames();
/**
- * Get the all request parameters
+ * Get all the request parameters. If there are multiple values for a parameter, it contains the first one.
*
* @return the parameters
*/
Map<String, String> getParameters();
/**
+ * Get all the values of all the request parameters.
+ *
+ * @return the parameters
+ */
+ Map<String, List<String>> getParametersValues();
+
+ /**
* Get a request header
*
- * @param name
+ * @param name header name
* @return the header or null if not found
*/
String getHeader(CharSequence name);
@@ -76,8 +83,8 @@ public interface IServletRequest {
/**
* Get a request header if found, return the default value, otherwise
*
- * @param name
- * @param defaultValue
+ * @param name header name
+ * @param defaultValue default value
* @return the header or defaultValue if not found
*/
default String getHeader(CharSequence name, String defaultValue) {
diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java
index f5749b1..d4f57ea 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/BaseRequest.java
@@ -19,14 +19,15 @@
package org.apache.hyracks.http.server;
import java.net.InetSocketAddress;
+import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.hyracks.http.api.IServletRequest;
-import org.apache.hyracks.http.server.utils.HttpUtil;
import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.codec.http.FullHttpRequest;
@@ -34,22 +35,24 @@ import io.netty.handler.codec.http.HttpScheme;
import io.netty.handler.codec.http.QueryStringDecoder;
public class BaseRequest implements IServletRequest {
+
+ private static final List<String> NO_PARAM = Collections.singletonList(null);
protected final FullHttpRequest request;
- protected final Map<String, List<String>> parameters;
+ protected final Map<? extends CharSequence, List<String>> parameters;
protected final InetSocketAddress remoteAddress;
protected final HttpScheme scheme;
protected final InetSocketAddress localAddress;
public static IServletRequest create(ChannelHandlerContext ctx, FullHttpRequest request, HttpScheme scheme) {
QueryStringDecoder decoder = new QueryStringDecoder(request.uri());
- Map<String, List<String>> param = decoder.parameters();
+ Map<? extends CharSequence, List<String>> param = decoder.parameters();
InetSocketAddress remoteAddress = (InetSocketAddress) ctx.channel().remoteAddress();
InetSocketAddress localAddress = (InetSocketAddress) ctx.channel().localAddress();
return new BaseRequest(request, localAddress, remoteAddress, param, scheme);
}
protected BaseRequest(FullHttpRequest request, InetSocketAddress localAddress, InetSocketAddress remoteAddress,
- Map<String, List<String>> parameters, HttpScheme scheme) {
+ Map<? extends CharSequence, List<String>> parameters, HttpScheme scheme) {
this.request = request;
this.localAddress = localAddress;
this.remoteAddress = remoteAddress;
@@ -64,28 +67,33 @@ public class BaseRequest implements IServletRequest {
@Override
public String getParameter(CharSequence name) {
- return HttpUtil.getParameter(parameters, name);
+ return parameters.getOrDefault(name, NO_PARAM).get(0);
}
@Override
public List<String> getParameterValues(CharSequence name) {
- List<String> values = parameters.get(String.valueOf(name));
- return values != null ? Collections.unmodifiableList(values) : null;
+ return Collections.unmodifiableList(parameters.getOrDefault(name, Collections.emptyList()));
}
@Override
public Set<String> getParameterNames() {
- return Collections.unmodifiableSet(parameters.keySet());
+ Set<String> names = new HashSet<>();
+ parameters.keySet().forEach(name -> names.add(name.toString()));
+ return names;
}
@Override
public Map<String, String> getParameters() {
HashMap<String, String> paramMap = new HashMap<>();
- for (String name : parameters.keySet()) {
- paramMap.put(name, HttpUtil.getParameter(parameters, name));
+ parameters.forEach((name, values) -> paramMap.put(name.toString(), values.get(0)));
+ return paramMap;
+ }
- }
- return Collections.unmodifiableMap(paramMap);
+ @Override
+ public Map<String, List<String>> getParametersValues() {
+ Map<String, List<String>> params = new HashMap<>();
+ parameters.forEach((name, values) -> params.put(name.toString(), new ArrayList<>(values)));
+ return params;
}
@Override
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 4f20174..053356f 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
@@ -24,7 +24,6 @@ import java.io.Reader;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@@ -68,11 +67,6 @@ public class HttpUtil {
private HttpUtil() {
}
- public static String getParameter(Map<String, List<String>> parameters, CharSequence name) {
- List<String> parameter = parameters.get(String.valueOf(name));
- return parameter == null ? null : String.join(",", parameter);
- }
-
public static IServletRequest toServletRequest(ChannelHandlerContext ctx, FullHttpRequest request,
HttpScheme scheme) {
return ContentType.APPLICATION_X_WWW_FORM_URLENCODED.equals(getContentTypeOnly(request))