Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add semicolonIsNormalChar and maxParams properties to the server config #11416

Open
wants to merge 4 commits into
base: 4.8.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import io.micronaut.http.netty.stream.StreamedHttpRequest;
import io.netty.handler.codec.http.DefaultFullHttpRequest;
import io.netty.handler.codec.http.DefaultLastHttpContent;
import io.netty.handler.codec.http.HttpConstants;
import io.netty.handler.codec.http.LastHttpContent;
import io.netty.handler.codec.http.QueryStringDecoder;
import io.netty.util.DefaultAttributeMap;
Expand Down Expand Up @@ -191,14 +192,29 @@ public String getPath() {
*/
protected abstract Charset initCharset(Charset characterEncoding);

/**
* @return the maximum number of parameters.
*/
protected abstract int getMaxParams();

/**
* @return {@code true} if yes, {@code false} otherwise.
*/
protected abstract boolean isSemicolonIsNormalChar();


/**
* @param uri The URI
* @return The query string decoder
*/
@SuppressWarnings("ConstantConditions")
protected final QueryStringDecoder createDecoder(URI uri) {
Charset cs = getCharacterEncoding();
return cs != null ? new QueryStringDecoder(uri, cs) : new QueryStringDecoder(uri);
boolean semicolonIsNormalChar = isSemicolonIsNormalChar();
int maxParams = getMaxParams();
return cs != null ?
new QueryStringDecoder(uri, cs, maxParams, semicolonIsNormalChar) :
new QueryStringDecoder(uri, HttpConstants.DEFAULT_CHARSET, maxParams, semicolonIsNormalChar);
}

private NettyHttpParameters decodeParameters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,16 @@ protected Charset initCharset(Charset characterEncoding) {
return characterEncoding == null ? serverConfiguration.getDefaultCharset() : characterEncoding;
}

@Override
protected int getMaxParams() {
return serverConfiguration.getMaxParams();
}

@Override
protected boolean isSemicolonIsNormalChar() {
return serverConfiguration.isSemicolonIsNormalChar();
}

/**
* @return Return true if the request is form data.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ public class HttpServerConfiguration implements ServerContextPathProvider {
*/
@SuppressWarnings("WeakerAccess")
public static final boolean DEFAULT_DISPATCH_OPTIONS_REQUESTS = false;

@SuppressWarnings("WeakerAccess")
public static final boolean DEFAULT_SEMICOLON_IS_NORMAL_CHAR = false;

@SuppressWarnings("WeakerAccess")
public static final int DEFAULT_MAX_PARAMS = 1024;
private Integer port;
private String host;
private Integer readTimeout;
Expand Down Expand Up @@ -155,6 +161,8 @@ public class HttpServerConfiguration implements ServerContextPathProvider {
private ThreadSelection threadSelection = ThreadSelection.MANUAL;
private boolean validateUrl = true;
private boolean notFoundOnMissingBody = true;
private boolean semicolonIsNormalChar = DEFAULT_SEMICOLON_IS_NORMAL_CHAR;
private int maxParams = DEFAULT_MAX_PARAMS;

/**
* Default constructor.
Expand Down Expand Up @@ -598,6 +606,45 @@ public void setNotFoundOnMissingBody(boolean notFoundOnMissingBody) {
this.notFoundOnMissingBody = notFoundOnMissingBody;
}

/**
* Returns whether the semicolon is considered a normal character in the query.
* A "normal" semicolon is one that is not used as a parameter separator.
*
* @return {@code true} if the semicolon is a normal character, {@code false} otherwise.
* @since 4.8
*/
public boolean isSemicolonIsNormalChar() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this belongs in HttpServerConfiguration since I am unsure the other server implementations (jetty, tomcat etc.) allow this kind of configuration. For the moment it should be moved to NettyHttpServerConfiguration. @yawkat WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

Copy link
Member

@yawkat yawkat Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm actually since this also affects our non-netty copy of QueryStringDecoder, I guess it's fine here

Copy link
Contributor Author

@glorrian glorrian Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@graemerocher AbstractNettyHttpRequest uses HttpServerConfig interface for a config field, that's why i wrote it there, but it seems strange to me too, i think it should use NettyHttpServerConfig interface due to strong AbstractNettyHttpRequest bind to Netty

return semicolonIsNormalChar;
}

/**
* Sets whether the semicolon should be considered a normal character in the query.
* A "normal" semicolon is one that is not used as a parameter separator.
*
* @param semicolonIsNormalChar {@code true} if the semicolon should be a normal character, {@code false} otherwise.
* @since 4.8
*/
public void setSemicolonIsNormalChar(boolean semicolonIsNormalChar) {
this.semicolonIsNormalChar = semicolonIsNormalChar;
}

/**
* @return the maximum parameter count.
* @since 4.8
*/
public int getMaxParams() {
return maxParams;
}

/**
* @param maxParams the maximum parameter count.
* @since 4.8
*/
public void setMaxParams(int maxParams) {
this.maxParams = maxParams;
}


/**
* Configuration for multipart handling.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,10 @@ public interface FormConfiguration {
* @return default maximum of decoded key value parameters. It defaults to 1024.
*/
int getMaxDecodedKeyValueParameters();

/**
* @return true if the semicolon handle as a normal character, false otherwise.
*/
boolean isSemicolonIsNormalChar();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to avoid a breaking change this should be a default method that returns false

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,15 @@ final class FormConfigurationProperties implements FormConfiguration {
@SuppressWarnings("WeakerAccess")
private static final int DEFAULT_MAX_DECODED_KEY_VALUE_PARAMETERS = 1024;

/**
* Default value indicating whether the semicolon is treated as a normal character
* used in {@link io.micronaut.http.form.FormUrlEncodedDecoder}.
*/
@SuppressWarnings("WeakerAccess")
private static final boolean DEFAULT_SEMICOLON_IS_NORMAL_CHAR = false;

private int maxDecodedKeyValueParameters = DEFAULT_MAX_DECODED_KEY_VALUE_PARAMETERS;
private boolean semicolonIsNormalChar = DEFAULT_SEMICOLON_IS_NORMAL_CHAR;

/**
*
Expand All @@ -45,11 +53,27 @@ public int getMaxDecodedKeyValueParameters() {
return maxDecodedKeyValueParameters;
}


/**
* @return true if the semicolon is treated as a normal character, false otherwise
*/
@Override
public boolean isSemicolonIsNormalChar() {
return semicolonIsNormalChar;
}

/**
* default maximum of decoded key value parameters. Default value {@link #DEFAULT_MAX_DECODED_KEY_VALUE_PARAMETERS}.
* @param maxDecodedKeyValueParameters default maximum of decoded key value parameters
*/
public void setMaxDecodedKeyValueParameters(int maxDecodedKeyValueParameters) {
this.maxDecodedKeyValueParameters = maxDecodedKeyValueParameters;
}

/**
* @param semicolonIsNormalChar true if the semicolon should be treated as a normal character, false otherwise
*/
public void setSemicolonIsNormalChar(boolean semicolonIsNormalChar) {
this.semicolonIsNormalChar = semicolonIsNormalChar;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any changes to DefaultFormUrlEncodedDecoder that would respect this configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

}
20 changes: 19 additions & 1 deletion http/src/main/java/io/micronaut/http/uri/QueryStringDecoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ final class QueryStringDecoder {
private final Charset charset;
private final String uri;
private final int maxParams;
private final boolean semicolonIsNormalChar;
private int pathEndIdx;
private String path;
private Map<String, List<String>> params;
Expand Down Expand Up @@ -121,9 +122,14 @@ final class QueryStringDecoder {
* @param maxParams The maximum number of params
*/
QueryStringDecoder(String uri, Charset charset, boolean hasPath, int maxParams) {
this(uri, charset, hasPath, maxParams, false);
}

QueryStringDecoder(String uri, Charset charset, boolean hasPath, int maxParams, boolean semicolonIsNormalChar) {
this.uri = Objects.requireNonNull(uri, "uri");
this.charset = Objects.requireNonNull(charset, "charset");
this.maxParams = maxParams;
this.semicolonIsNormalChar = semicolonIsNormalChar;

// `-1` means that path end index will be initialized lazily
pathEndIdx = hasPath ? -1 : 0;
Expand Down Expand Up @@ -159,13 +165,18 @@ final class QueryStringDecoder {
* @param maxParams The maximum number of params
*/
QueryStringDecoder(URI uri, Charset charset, int maxParams) {
this(uri, charset, maxParams, false);
}

QueryStringDecoder(URI uri, Charset charset, int maxParams, boolean semicolonIsNormalChar) {
String rawPath = uri.getRawPath();
if (rawPath == null) {
rawPath = EMPTY_STRING;
}
this.uri = uriToString(uri);
this.charset = Objects.requireNonNull(charset, "charset");
this.maxParams = ArgumentUtils.requirePositive("maxParams", maxParams);
this.semicolonIsNormalChar = semicolonIsNormalChar;
pathEndIdx = rawPath.length();
}

Expand Down Expand Up @@ -196,7 +207,7 @@ public String path() {
*/
public Map<String, List<String>> parameters() {
if (params == null) {
params = decodeParams(uri, pathEndIdx(), charset, maxParams);
params = decodeParams(uri, pathEndIdx(), charset, maxParams, semicolonIsNormalChar);
}
return params;
}
Expand Down Expand Up @@ -260,6 +271,10 @@ private static String uriToString(URI uri) {
}

private static Map<String, List<String>> decodeParams(String s, int from, Charset charset, int paramsLimit) {
return decodeParams(s, from, charset, paramsLimit, false);
}

private static Map<String, List<String>> decodeParams(String s, int from, Charset charset, int paramsLimit, boolean semicolonIsNormalChar) {
int len = s.length();
if (from >= len) {
return Collections.emptyMap();
Expand All @@ -283,6 +298,9 @@ private static Map<String, List<String>> decodeParams(String s, int from, Charse
break;
case '&':
case ';':
if (semicolonIsNormalChar) {
continue;
}
if (addParam(s, nameStart, valueStart, i, params, charset)) {
paramsLimit--;
if (paramsLimit == 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;

@MicronautTest(startApplication = false)
class FormConfigurationTest {
Expand All @@ -17,4 +18,9 @@ class FormConfigurationTest {
void defaultMaxParams() {
assertEquals(1024, formConfiguration.getMaxDecodedKeyValueParameters());
}

@Test
void defaultSemicolonIsNormalChar() {
assertFalse(formConfiguration.isSemicolonIsNormalChar());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

@Property(name = "micronaut.http.forms.max-decoded-key-value-parameters", value = "512")
@Property(name = "micronaut.http.forms.semicolon-is-normal-char", value = "true")
@MicronautTest(startApplication = false)
class FormConfigurationViaPropertyTest {

Expand All @@ -19,4 +21,9 @@ class FormConfigurationViaPropertyTest {
void maxParamCanBeSetViaProperty() {
assertEquals(512, formConfiguration.getMaxDecodedKeyValueParameters());
}

@Test
void semicolonIsNormalCharCanBeSetViaProperty() {
assertTrue(formConfiguration.isSemicolonIsNormalChar());
}
}
Loading