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.

Sign up for GitHub

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

Jump to bottom

TASK-5888 - REST Error : 414 URI too large and 431 Header too large #2443

Merged
merged 3 commits into from
May 17, 2024
Merged

TASK-5888 - REST Error : 414 URI too large and 431 Header too large #2443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
390f787
server: Add server.rest.httpConfiguraiton to configuration.yml #TAKS-…
j-coll May 8, 2024
f661aeb
server: Extract individual init methods at RestServer #TASK-5888
j-coll May 8, 2024
9aae8c1
core: Fix typo on unused configuration field. #TASK-5888
j-coll May 8, 2024
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
13 changes: 4 additions & 9 deletions opencga-core/src/main/java/org/opencb/opencga/core/config/AbstractServerConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@

package org.opencb.opencga.core.config;


/**
* Created by imedina on 25/04/16.
*/
public abstract class AbstractServerConfiguration {

protected int port;
protected String logFile;

public AbstractServerConfiguration() {
}
Expand All @@ -35,7 +35,6 @@ public AbstractServerConfiguration(int port) {
public String toString() {
final StringBuilder sb = new StringBuilder("ServerConfiguration{");
sb.append("port=").append(port);
sb.append(", logFile='").append(logFile).append('\'');
sb.append('}');
return sb.toString();
}
Expand All @@ -49,13 +48,9 @@ public AbstractServerConfiguration setPort(int port) {
return this;
}

public String getLogFile() {
return logFile;
}

public AbstractServerConfiguration setLogFile(String logFile) {
this.logFile = logFile;
return this;
@Deprecated
protected void setLogFile(Object o) {
Configuration.reportUnusedField("configuration.yml#server.[rest|grpc].logFile", o);
Copy link
Member

Choose a reason for hiding this comment

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

I like this :)

}

}
16 changes: 13 additions & 3 deletions opencga-core/src/main/java/org/opencb/opencga/core/config/Configuration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ public class Configuration {

private static Logger logger;

private static final Set<String> reportedFields = new HashSet<>();

private static final String DEFAULT_CONFIGURATION_FORMAT = "yaml";

static {
Expand Down Expand Up @@ -200,6 +202,16 @@ private static void overwriteWithEnvironmentVariables(Configuration configuratio
}
}

public static void reportUnusedField(String field, Object value) {
Copy link
Member

Choose a reason for hiding this comment

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

I'll definitely be using this method

// Report only if the value is not null and not an empty string
if (value != null && !(value instanceof String && ((String) value).isEmpty())) {
if (reportedFields.add(field)) {
// Only log the first time a field is found
logger.warn("Ignored configuration option '{}' with value '{}'. The option was deprecated and removed.", field, value);
}
}
}

@Override
public String toString() {
final StringBuilder sb = new StringBuilder("Configuration{");
Expand Down Expand Up @@ -246,9 +258,7 @@ public String getLogFile() {

@Deprecated
public Configuration setLogFile(String logFile) {
if (logFile != null) {
logger.warn("Deprecated option 'configuration.yml#logFile'");
}
reportUnusedField("configuration.yml#logFile", logFile);
return this;
}

Expand Down
122 changes: 101 additions & 21 deletions opencga-core/src/main/java/org/opencb/opencga/core/config/RestServerConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,52 +16,132 @@

package org.opencb.opencga.core.config;

import java.util.Objects;

/**
* Created by imedina on 22/05/16.
*/
public class RestServerConfiguration extends AbstractServerConfiguration {

private int defaultLimit;
private int maxLimit;


public HttpConfiguration httpConfiguration = new HttpConfiguration();
public RestServerConfiguration() {
}

public RestServerConfiguration(int port) {
this(port, 2000, 5000);
}

public RestServerConfiguration(int port, int defaultLimit, int maxLimit) {
super(port);
this.defaultLimit = defaultLimit;
this.maxLimit = maxLimit;
}

@Override
public String toString() {
final StringBuilder sb = new StringBuilder("RestServerConfiguration{");
sb.append("defaultLimit=").append(defaultLimit);
sb.append(", maxLimit=").append(maxLimit);
sb.append("port=").append(port);
sb.append(", httpConfiguration=").append(httpConfiguration);
sb.append('}');
return sb.toString();
}

public int getDefaultLimit() {
return defaultLimit;
@Deprecated
protected void setDefaultLimit(Object o) {
Configuration.reportUnusedField("configuration.yml#server.rest.defaultLimit", o);
}

public RestServerConfiguration setDefaultLimit(int defaultLimit) {
this.defaultLimit = defaultLimit;
return this;
@Deprecated
protected void setMaxLimit(Object o) {
Configuration.reportUnusedField("configuration.yml#server.rest.maxLimit", o);
}

public int getMaxLimit() {
return maxLimit;
public HttpConfiguration getHttpConfiguration() {
return httpConfiguration;
}

public RestServerConfiguration setMaxLimit(int maxLimit) {
this.maxLimit = maxLimit;
public RestServerConfiguration setHttpConfiguration(HttpConfiguration httpConfiguration) {
this.httpConfiguration = httpConfiguration;
return this;
}

public static class HttpConfiguration {
private int outputBufferSize = -1;
private int outputAggregationSize = -1;
private int requestHeaderSize = -1;
private int responseHeaderSize = -1;
private int headerCacheSize = -1;

public int getOutputBufferSize() {
return outputBufferSize;
}

public HttpConfiguration setOutputBufferSize(int outputBufferSize) {
this.outputBufferSize = outputBufferSize;
return this;
}

public int getOutputAggregationSize() {
return outputAggregationSize;
}

public HttpConfiguration setOutputAggregationSize(int outputAggregationSize) {
this.outputAggregationSize = outputAggregationSize;
return this;
}

public int getRequestHeaderSize() {
return requestHeaderSize;
}

public HttpConfiguration setRequestHeaderSize(int requestHeaderSize) {
this.requestHeaderSize = requestHeaderSize;
return this;
}

public int getResponseHeaderSize() {
return responseHeaderSize;
}

public HttpConfiguration setResponseHeaderSize(int responseHeaderSize) {
this.responseHeaderSize = responseHeaderSize;
return this;
}

public int getHeaderCacheSize() {
return headerCacheSize;
}

public HttpConfiguration setHeaderCacheSize(int headerCacheSize) {
this.headerCacheSize = headerCacheSize;
return this;
}

@Override
public String toString() {
final StringBuilder sb = new StringBuilder("HttpConfiguration{");
sb.append("outputBufferSize=").append(outputBufferSize);
sb.append(", outputAggregationSize=").append(outputAggregationSize);
sb.append(", requestHeaderSize=").append(requestHeaderSize);
sb.append(", responseHeaderSize=").append(responseHeaderSize);
sb.append(", headerCacheSize=").append(headerCacheSize);
sb.append('}');
return sb.toString();
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
HttpConfiguration that = (HttpConfiguration) o;
return outputBufferSize == that.outputBufferSize &&
outputAggregationSize == that.outputAggregationSize &&
requestHeaderSize == that.requestHeaderSize &&
responseHeaderSize == that.responseHeaderSize &&
headerCacheSize == that.headerCacheSize;
}

@Override
public int hashCode() {
return Objects.hash(outputBufferSize, outputAggregationSize, requestHeaderSize, responseHeaderSize, headerCacheSize);
}
}
}
16 changes: 12 additions & 4 deletions opencga-core/src/main/resources/configuration.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,20 @@ authentication:
server:
rest:
port: ${OPENCGA.SERVER.REST.PORT}
logFile: null
defaultLimit: 2000
maxLimit: 5000
httpConfiguration:
# The size in bytes of the output buffer used to aggregate HTTP output
outputBufferSize: 32768
# The maximum size in bytes for HTTP output to be aggregated
outputAggregationSize: 8192
# The maximum allowed size in bytes for a HTTP request header
requestHeaderSize: 8192
# The maximum allowed size in bytes for a HTTP response header
responseHeaderSize: 8192
# The maximum allowed size in bytes for a HTTP header field cache
headerCacheSize: 4096

grpc:
port: ${OPENCGA.SERVER.GRPC.PORT}
logFile: null

optimizations:
simplifyPermissions: ${OPENCGA_OPTIMIZATIONS_SIMPLIFY_PERMISSIONS}
Expand Down
78 changes: 8 additions & 70 deletions opencga-core/src/test/java/org/opencb/opencga/core/config/ConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@

package org.opencb.opencga.core.config;

import org.apache.commons.lang3.StringUtils;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.opencb.opencga.core.testclassification.duration.ShortTests;

import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.text.SimpleDateFormat;
import java.util.*;

/**
Expand All @@ -34,68 +34,7 @@
public class ConfigurationTest {

@Test
public void test() {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder who left this here...

String path1 = "a/b/c.txt";
String path2 = "a/b/c/";
String path3 = "p.txt";

System.out.println(path1 + " ---- " + getParentPath(path1));
System.out.println(path2 + " ---- " + getParentPath(path2));
System.out.println(path3 + " ---- " + getParentPath(path3));

System.out.println(path1 + " ---- " + calculateAllPossiblePaths(path1));
System.out.println(path2 + " ---- " + calculateAllPossiblePaths(path2));
System.out.println(path3 + " ---- " + calculateAllPossiblePaths(path3));
System.out.println("'' ---- " + calculateAllPossiblePaths(""));

System.out.println(path1 + " ---- " + getFileName(path1));
System.out.println(path2 + " ---- " + getFileName(path2));
System.out.println(path3 + " ---- " + getFileName(path3));
System.out.println("'' ---- " + getFileName(""));

}

String getParentPath(String strPath) {
Path path = Paths.get(strPath);
Path parent = path.getParent();
if (parent != null) {
return parent.toString() + "/";
} else {
return "";
}
}

String getFileName(String strPath) {
if (StringUtils.isEmpty(strPath)) {
return ".";
}
return Paths.get(strPath).getFileName().toString();
}

public static List<String> calculateAllPossiblePaths(String filePath) {
if (StringUtils.isEmpty(filePath) || "/".equals(filePath)) {
return Collections.singletonList("");
}
StringBuilder pathBuilder = new StringBuilder();
String[] split = filePath.split("/");
List<String> paths = new ArrayList<>(split.length + 1);
paths.add(""); //Add study root folder
//Add intermediate folders
//Do not add the last split, could be a file or a folder..
//Depending on this, it could end with '/' or not.
for (int i = 0; i < split.length - 1; i++) {
String f = split[i];
System.out.println(f);
pathBuilder = new StringBuilder(pathBuilder.toString()).append(f).append("/");
paths.add(pathBuilder.toString());
}
paths.add(filePath); //Add the file path
return paths;
}


@Test
public void testDefault() {
public void testDefault() throws IOException {
Configuration configuration = new Configuration();

configuration.setLogLevel("INFO");
Expand Down Expand Up @@ -136,7 +75,7 @@ public void testDefault() {
configuration.setAudit(audit);

ServerConfiguration serverConfiguration = new ServerConfiguration();
RestServerConfiguration rest = new RestServerConfiguration(1000, 100, 1000);
RestServerConfiguration rest = new RestServerConfiguration(1000);
GrpcServerConfiguration grpc = new GrpcServerConfiguration(1001);
serverConfiguration.setGrpc(grpc);
serverConfiguration.setRest(rest);
Expand All @@ -155,11 +94,10 @@ public void testDefault() {
// catalogConfiguration.getStorageEngines().add(storageEngineConfiguration1);
// catalogConfiguration.getStorageEngines().add(storageEngineConfiguration2);

try {
configuration.serialize(new FileOutputStream("/tmp/configuration-test.yml"));
} catch (IOException e) {
e.printStackTrace();
}
Path outdir = Paths.get("target/test-data", "junit-opencga-" +
new SimpleDateFormat("yyyy-MM-dd_HH-mm-ss.SSS").format(new Date()));
Files.createDirectories(outdir);
configuration.serialize(Files.newOutputStream(outdir.resolve("configuration-test.yml").toFile().toPath()));
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion opencga-core/src/test/resources/configuration-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ authentication:
server:
rest:
port: 8080
logFile: null
logFile: "some_file_but_this_field_is_deprecated"
defaultLimit: 2000
maxLimit: 5000

Expand Down
Loading
Loading