Skip to content

Commit

Permalink
fix: dataquery cannot correctly process query strings
Browse files Browse the repository at this point in the history
The DataQuery expects a decoded query string in the setQuery method and
in the constructors that accept a query parameter. These methods are
now deprecated since this cannot be fixed. A new method setRawQuery is
introduced which accepts the encoded query string and creates the
parameters from it.
  • Loading branch information
halber committed Nov 9, 2022
1 parent cb1be0e commit 4cccf60
Show file tree
Hide file tree
Showing 9 changed files with 196 additions and 23 deletions.
117 changes: 111 additions & 6 deletions src/main/java/io/neonbee/data/DataQuery.java
Expand Up @@ -6,6 +6,9 @@
import static java.util.stream.Collectors.mapping;
import static java.util.stream.Collectors.toCollection;

import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -121,68 +124,92 @@ public DataQuery(DataAction action, String uriPath, Buffer body) {
/**
* DataQuery with a URI path and query.
*
* @deprecated This constructor is deprecated because it cannot correctly process query strings that contain the
* '&' or '=' characters in the value. Eg. {@code filter=string eq '&'}
*
* @param uriPath The URI path to request
* @param query The (URL decoded) query string
*/
@Deprecated
public DataQuery(String uriPath, String query) {
this(READ, uriPath, query);
}

/**
* DataQuery with a URI path and query.
*
* @deprecated This constructor is deprecated because it cannot correctly process query strings that contain the
* '&' or '=' characters in the value. Eg. {@code filter=string eq '&'}
*
* @param action The action to perform
* @param uriPath The URI path to request
* @param query The (URL decoded) query string
*/
@Deprecated
public DataQuery(DataAction action, String uriPath, String query) {
this(action, uriPath, query, (Buffer) null);
}

/**
* DataQuery with a URI path and query.
*
* @deprecated This constructor is deprecated because it cannot correctly process query strings that contain the
* '&' or '=' characters in the value. Eg. {@code filter=string eq '&'}
*
* @param action The action to perform
* @param uriPath The URI path to request
* @param query The (URL decoded) query string
* @param body The body of this query
*/
@Deprecated
public DataQuery(DataAction action, String uriPath, String query, Buffer body) {
this(action, uriPath, query, null, body);
}

/**
* DataQuery with a URI path, query and headers.
*
* @deprecated This constructor is deprecated because it cannot correctly process query strings that contain the
* '&' or '=' characters in the value. Eg. {@code filter=string eq '&'}
*
* @param uriPath The URI path to request
* @param query The (URL decoded) query string
* @param headers The headers of this query
*/
@Deprecated
public DataQuery(String uriPath, String query, Map<String, List<String>> headers) {
this(READ, uriPath, query, headers);
}

/**
* DataQuery with a URI path, query and headers.
*
* @deprecated This constructor is deprecated because it cannot correctly process query strings that contain the
* '&amp;' or '=' characters in the value. Eg. {@code filter=string eq '&'}
*
* @param action The action to perform
* @param uriPath The URI path to request
* @param query The (URL decoded) query string
* @param headers The headers of this query
*/
@Deprecated
public DataQuery(DataAction action, String uriPath, String query, Map<String, List<String>> headers) {
this(action, uriPath, query, headers, null);
}

/**
* DataQuery with a URI path, query and headers.
*
* @deprecated This constructor is deprecated because it cannot correctly process query strings that contain the
* '&amp;' or '=' characters in the value. Eg. {@code filter=string eq '&'}
*
* @param action The action to perform
* @param uriPath The URI path to request
* @param query The (URL decoded) query string
* @param headers The headers of this query
* @param body The body of this query
*/
@Deprecated
public DataQuery(DataAction action, String uriPath, String query, Map<String, List<String>> headers, Buffer body) {
this.action = action;
this.setUriPath(uriPath); // Calling the setter here, because there is an additional check implemented.
Expand All @@ -191,6 +218,24 @@ public DataQuery(DataAction action, String uriPath, String query, Map<String, Li
this.body = CollectionHelper.copyOf(body);
}

/**
* DataQuery with a URI path, query and headers.
*
* @param action The action to perform
* @param uriPath The URI path to request
* @param queryParameters The query parameters of this query
* @param headers The headers of this query
* @param body The body of this query
*/
public DataQuery(DataAction action, String uriPath, Map<String, List<String>> queryParameters,
Map<String, List<String>> headers, Buffer body) {
this.action = action;
this.setUriPath(uriPath); // Calling the setter here, because there is an additional check implemented.
this.headers = CollectionHelper.mutableCopyOf(headers != null ? headers : Map.of());
this.parameters = CollectionHelper.mutableCopyOf(queryParameters != null ? queryParameters : Map.of());
this.body = CollectionHelper.copyOf(body);
}

/**
* Returns the {@link DataAction} of this data query.
*
Expand Down Expand Up @@ -235,29 +280,63 @@ public DataQuery setUriPath(String uriPath) {
return this;
}

/**
* Returns the parameters of this data query represented as a URL encoded string.
*
* @return the URL encoded query string
*/
public String getRawQuery() {
return getQuery(s -> URLEncoder.encode(s, StandardCharsets.UTF_8).replace("+", "%20"));
}

/**
* Returns the parameters of this data query represented as a string.
*
* @deprecated This method is deprecated because it can generate query strings that can no longer be parsed
* correctly. See {@link DataQuery#setQuery}
*
* @return the query
*/
@Deprecated
public String getQuery() {
Function<String, Stream<String>> paramBuilder =
name -> getParameterValues(name).stream().map(value -> String.format("%s=%s", name, value));
return getQuery(Function.identity());
}

private String getQuery(Function<String, String> encoder) {
Function<String, Stream<String>> paramBuilder = name -> getParameterValues(name).stream()
.map(value -> String.format("%s=%s", encoder.apply(name), encoder.apply(value)));

return parameters.keySet().stream().flatMap(paramBuilder).collect(joining("&"));
}

/**
* Set the query string. Please note that this will also remove all previously added parameters.
*
* <b>This method is deprecated because it cannot correctly process query strings that contain the '&amp;' or '='
* characters in the value.</b><br>
* Eg. {@code filter=string eq '&'}
*
* @param query the (URL decoded) query to set
* @return the DataQuery for chaining
*/
@Deprecated
public DataQuery setQuery(String query) {
this.parameters = parseQueryString(query);
return this;
}

/**
* Set the query string. Please note that this will also remove all previously added parameters.
*
* @param encodedQuery the (URL encoded) query to set
* @return the DataQuery for chaining
* @throws IllegalArgumentException if the implementation encounters illegal characters
*/
public DataQuery setRawQuery(String encodedQuery) {
this.parameters = parseEncodedQueryString(encodedQuery);
return this;
}

/**
* Returns the parameters of this data query.
*
Expand Down Expand Up @@ -336,14 +415,40 @@ public DataQuery removeParameter(String name) {
return this;
}

/**
* Pars the encoded query parameter string.
*
* @param encodedQuery the encoded query parameter string
* @return the query parameter map
* @throws IllegalArgumentException if the implementation encounters illegal characters
*/
public static Map<String, List<String>> parseEncodedQueryString(String encodedQuery) {
return parseQueryString(encodedQuery, s -> URLDecoder.decode(s, StandardCharsets.UTF_8));
}

/**
* This method is deprecated because it cannot correctly process query strings that contain the '&amp;' or '='
* characters in the value.<br>
* Eg. {@code filter=string eq '&'}
*
* @param query the decoded query string
* @return map with the query
*/
@VisibleForTesting
@Deprecated
static Map<String, List<String>> parseQueryString(String query) {
if (Strings.isNullOrEmpty(query)) {
return parseQueryString(query, Function.identity());
}

private static Map<String, List<String>> parseQueryString(String encodedQuery, Function<String, String> decoder) {
if (Strings.isNullOrEmpty(encodedQuery)) {
return new HashMap<>();
}

return QUERY_SPLIT_PATTERN.splitAsStream(query).map(paramString -> PARAM_SPLIT_PATTERN.split(paramString, 2))
.map(paramArray -> Map.entry(paramArray[0], paramArray.length > 1 ? paramArray[1] : ""))
return QUERY_SPLIT_PATTERN.splitAsStream(encodedQuery)
.map(paramString -> PARAM_SPLIT_PATTERN.split(paramString, 2))
.map(paramArray -> Map.entry(decoder.apply(paramArray[0]),
paramArray.length > 1 ? decoder.apply(paramArray[1]) : ""))
.collect(groupingBy(Map.Entry::getKey, HashMap::new,
mapping(Map.Entry::getValue, toCollection(ArrayList::new))));
}
Expand Down Expand Up @@ -450,7 +555,7 @@ public DataQuery setBody(Buffer body) {
* @return a copy of this DataQuery
*/
public DataQuery copy() {
return new DataQuery(action, uriPath, getQuery(), headers, body);
return new DataQuery(action, uriPath, parameters, headers, body);
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/neonbee/data/DataVerticle.java
Expand Up @@ -494,10 +494,10 @@ public Future<T> retrieveData(DataQuery query, DataMap require, DataContext cont

private <U> void reportRequestDataMetrics(DataRequest request, Future<U> future) {
List<Tag> tags;
if (request.getQuery() == null || request.getQuery().getQuery() == null) {
if (request.getQuery() == null) {
tags = List.of();
} else {
tags = List.of(new ImmutableTag("query", request.getQuery().getQuery()));
tags = List.of(new ImmutableTag("query", request.getQuery().getRawQuery()));
}
String qualifiedName = request.getQualifiedName();

Expand Down
@@ -1,10 +1,9 @@
package io.neonbee.endpoint.odatav4.internal.olingo.processor;

import static com.google.common.base.Strings.nullToEmpty;
import static io.neonbee.entity.EntityVerticle.requestEntity;
import static java.net.URLDecoder.decode;
import static java.nio.charset.StandardCharsets.UTF_8;

import java.util.List;
import java.util.Map;
import java.util.Optional;

import org.apache.olingo.commons.api.data.Entity;
Expand Down Expand Up @@ -56,8 +55,8 @@ private static DataQuery odataRequestToQuery(ODataRequest request, DataAction ac
// the uriPath without /odata root path and without query path
String uriPath = "/" + request.getRawServiceResolutionUri() + request.getRawODataPath();
// the raw query path
String rawQueryPath = decode(nullToEmpty(request.getRawQueryPath()), UTF_8);
return new DataQuery(action, uriPath, rawQueryPath, request.getAllHeaders(), body).addHeader("X-HTTP-Method",
Map<String, List<String>> stringListMap = DataQuery.parseEncodedQueryString(request.getRawQueryPath());
return new DataQuery(action, uriPath, stringListMap, request.getAllHeaders(), body).addHeader("X-HTTP-Method",
request.getMethod().name());
}

Expand Down
12 changes: 5 additions & 7 deletions src/main/java/io/neonbee/endpoint/raw/RawEndpoint.java
Expand Up @@ -24,9 +24,9 @@
import static io.vertx.ext.web.impl.Utils.pathOffset;
import static java.lang.Character.isUpperCase;

import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.regex.Pattern;

Expand Down Expand Up @@ -131,19 +131,17 @@ public void handle(RoutingContext routingContext) {
return;
}

String decodedQueryPath = null;
Map<String, List<String>> queryParameterMap;
try {
if (!Strings.isNullOrEmpty(request.query())) {
decodedQueryPath = URLDecoder.decode(request.query(), StandardCharsets.UTF_8);
}
queryParameterMap = DataQuery.parseEncodedQueryString(request.query());
} catch (IllegalArgumentException e) {
routingContext.fail(BAD_REQUEST.code(), new IllegalArgumentException("Invalid request query"));
return;
}

DataQuery query = new DataQuery(action,
pathOffset(routingContext.normalizedPath(), routingContext).substring(qualifiedName.length() + 1),
decodedQueryPath, multiMapToMap(request.headers()), routingContext.body().buffer())
queryParameterMap, multiMapToMap(request.headers()), routingContext.body().buffer())
.addHeader("X-HTTP-Method", request.method().name());

DataContextImpl context = new DataContextImpl(routingContext);
Expand Down

0 comments on commit 4cccf60

Please sign in to comment.