goiri commented on code in PR #5382:
URL: https://github.com/apache/hadoop/pull/5382#discussion_r1110104339
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/RouterServerUtil.java:
##########
@@ -624,4 +636,109 @@ public static ReservationDefinition
convertReservationDefinition(
return definition;
}
+
+ /**
+ * Checks if the ApplicationSubmissionContext submitted with the application
+ * is valid.
+ *
+ * Current checks:
+ * - if its size is within limits.
+ *
+ * @param appContext the app context to check.
+ * @throws IOException if an IO error occurred.
+ * @throws YarnException yarn exception.
+ */
+ @Public
+ @Unstable
+ public static void
checkAppSubmissionContext(ApplicationSubmissionContextPBImpl appContext,
+ Configuration conf) throws IOException, YarnException {
+ // Prevents DoS over the ApplicationClientProtocol by checking the context
+ // the application was submitted with for any excessively large fields.
+ long maxAscSize =
conf.getLong(YarnConfiguration.ROUTER_ASC_INTERCEPTOR_MAX_SIZE,
+ YarnConfiguration.DEFAULT_ROUTER_ASC_INTERCEPTOR_MAX_SIZE);
+ if (appContext != null) {
+ int size = appContext.getProto().getSerializedSize();
+ if (size >= maxAscSize) {
+ logContainerLaunchContext(appContext);
+ String errMsg = "The size of the ApplicationSubmissionContext of the
application " +
+ appContext.getApplicationId() + " is above the limit. Size= " +
size;
+ throw new YarnException(errMsg);
+ }
+ }
+ }
+
+ /**
+ * Private helper for checkAppSubmissionContext that logs the fields in the
+ * context for debugging.
+ *
+ * @param appContext the app context.
+ * @throws IOException if an IO error occurred.
+ */
+ @Private
+ @Unstable
+ private static void
logContainerLaunchContext(ApplicationSubmissionContextPBImpl appContext)
+ throws IOException {
+ if (appContext != null && appContext.getAMContainerSpec() != null) {
Review Comment:
```
if (appContext == null || appContext.getAMContainerSpec() == null) {
return;
}
```
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml:
##########
@@ -5117,6 +5117,16 @@
</description>
</property>
+ <property>
+ <name>yarn.router.asc-interceptor-max-size</name>
+ <value>1048576</value>
+ <description>
+ We define the size limit of ApplicationSubmissionContext,
+ If the size of the ApplicationSubmissionContext is larger than this
value,
+ We will throw an exception. the default value is 1MB, which is 1048576
Bytes.
+ </description>
+ </property>
+
Review Comment:
This complains about blanks.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/RouterServerUtil.java:
##########
@@ -624,4 +636,109 @@ public static ReservationDefinition
convertReservationDefinition(
return definition;
}
+
+ /**
+ * Checks if the ApplicationSubmissionContext submitted with the application
+ * is valid.
+ *
+ * Current checks:
+ * - if its size is within limits.
+ *
+ * @param appContext the app context to check.
+ * @throws IOException if an IO error occurred.
+ * @throws YarnException yarn exception.
+ */
+ @Public
+ @Unstable
+ public static void
checkAppSubmissionContext(ApplicationSubmissionContextPBImpl appContext,
+ Configuration conf) throws IOException, YarnException {
+ // Prevents DoS over the ApplicationClientProtocol by checking the context
+ // the application was submitted with for any excessively large fields.
+ long maxAscSize =
conf.getLong(YarnConfiguration.ROUTER_ASC_INTERCEPTOR_MAX_SIZE,
+ YarnConfiguration.DEFAULT_ROUTER_ASC_INTERCEPTOR_MAX_SIZE);
+ if (appContext != null) {
+ int size = appContext.getProto().getSerializedSize();
+ if (size >= maxAscSize) {
+ logContainerLaunchContext(appContext);
+ String errMsg = "The size of the ApplicationSubmissionContext of the
application " +
+ appContext.getApplicationId() + " is above the limit. Size= " +
size;
+ throw new YarnException(errMsg);
+ }
+ }
+ }
+
+ /**
+ * Private helper for checkAppSubmissionContext that logs the fields in the
+ * context for debugging.
+ *
+ * @param appContext the app context.
+ * @throws IOException if an IO error occurred.
+ */
+ @Private
+ @Unstable
+ private static void
logContainerLaunchContext(ApplicationSubmissionContextPBImpl appContext)
+ throws IOException {
+ if (appContext != null && appContext.getAMContainerSpec() != null) {
+ ContainerLaunchContext launchContext = appContext.getAMContainerSpec();
+ ContainerLaunchContextPBImpl clc = (ContainerLaunchContextPBImpl)
launchContext;
Review Comment:
Do we need to check instanceof?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
