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

fix(url-encoding): fix regression in url encoding #12601

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 4 additions & 0 deletions docs/how/updating-datahub.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ This file documents any backwards-incompatible changes in DataHub and assists pe

- #12408: The `platform` field in the DataPlatformInstance GraphQL type is removed. Clients need to retrieve the platform via the optional `dataPlatformInstance` field.

### Known Issues

- #12601: Jetty 12 introduces a stricter handling of url encoding. We are currently applying a workaround to prevent a regression, while technically breaking the official specifications.

### Potential Downtime

### Deprecations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

import com.linkedin.metadata.spring.YamlPropertySourceFactory;
import java.lang.management.ManagementFactory;
import java.util.Set;
import javax.management.MBeanServer;
import org.eclipse.jetty.ee10.servlet.ServletHandler;
import org.eclipse.jetty.http.UriCompliance;
import org.eclipse.jetty.jmx.MBeanContainer;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.HttpConfiguration;
Expand Down Expand Up @@ -65,13 +68,27 @@
@Bean
public WebServerFactoryCustomizer<JettyServletWebServerFactory> jettyCustomizer() {
return factory -> {

// Configure HTTP
factory.addServerCustomizers(
server -> {

// HTTP Configuration
HttpConfiguration httpConfig = new HttpConfiguration();
httpConfig.setRequestHeaderSize(32768);

// See https://github.com/jetty/jetty.project/issues/11890
// Configure URI compliance to allow encoded slashes
httpConfig.setUriCompliance(
UriCompliance.from(
Set.of(

Check warning on line 84 in metadata-service/war/src/main/java/com/linkedin/gms/CommonApplicationConfig.java

View check run for this annotation

Codecov / codecov/patch

metadata-service/war/src/main/java/com/linkedin/gms/CommonApplicationConfig.java#L82-L84

Added lines #L82 - L84 were not covered by tests
UriCompliance.Violation.AMBIGUOUS_PATH_SEPARATOR,
UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING)));
// set this for Servlet 6+
server
.getContainedBeans(ServletHandler.class)
.forEach(handler -> handler.setDecodeAmbiguousURIs(true));

Check warning on line 90 in metadata-service/war/src/main/java/com/linkedin/gms/CommonApplicationConfig.java

View check run for this annotation

Codecov / codecov/patch

metadata-service/war/src/main/java/com/linkedin/gms/CommonApplicationConfig.java#L88-L90

Added lines #L88 - L90 were not covered by tests

// HTTP Connector
ServerConnector connector =
new ServerConnector(server, new HttpConnectionFactory(httpConfig));
Expand Down
98 changes: 98 additions & 0 deletions smoke-test/tests/openapi/v3/entities.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@
"method": "delete"
}
},
{
"request": {
"url": "/openapi/v3/entity/dataset/urn%3Ali%3Adataset%3A%28urn%3Ali%3AdataPlatform%3Atest%2Cdataset%2FEntityV3%2CPROD%29",
"description": "Remove test dataset with %2F",
"method": "delete"
}
},
{
"request": {
"url": "/openapi/v3/entity/dataset",
Expand Down Expand Up @@ -156,5 +163,96 @@
]
}
}
},
{
"request": {
"url": "/openapi/v3/entity/dataset",
"params": {
"async": "false"
},
"description": "Create dataset with %2F",
"json": [
{
"urn": "urn:li:dataset:(urn:li:dataPlatform:test,dataset/EntityV3,PROD)",
"datasetProperties": {
"value": {
"name": "dataset/EntityV3",
"qualifiedName": "entities.dataset/EntityV3",
"customProperties": {},
"tags": []
}
},
"status": {
"value": {
"removed": false
}
}
}
]
}
},
{
"request": {
"url": "/openapi/v3/entity/dataset/urn%3Ali%3Adataset%3A%28urn%3Ali%3AdataPlatform%3Atest%2Cdataset%2FEntityV3%2CPROD%29",
"method": "get",
"description": "Get dataset with %2F",
"json": [
{
"urn": "urn:li:dataset:(urn:li:dataPlatform:test,dataset/EntityV3,PROD)",
"datasetProperties": {
"value": {
"name": "dataset/EntityV3",
"qualifiedName": "entities.dataset/EntityV3",
"customProperties": {},
"tags": []
}
},
"status": {
"value": {
"removed": false
}
}
}
]
},
"response": {
"json": {
"urn": "urn:li:dataset:(urn:li:dataPlatform:test,dataset/EntityV3,PROD)",
"browsePathsV2": {
"value": {
"path": [
{
"id": "Default"
}
]
}
},
"datasetKey": {
"value": {
"name": "dataset/EntityV3",
"platform": "urn:li:dataPlatform:test",
"origin": "PROD"
}
},
"dataPlatformInstance": {
"value": {
"platform": "urn:li:dataPlatform:test"
}
},
"datasetProperties": {
"value": {
"name": "dataset/EntityV3",
"customProperties": {},
"qualifiedName": "entities.dataset/EntityV3",
"tags": []
}
},
"status": {
"value": {
"removed": false
}
}
}
}
}
]
Loading