[FLINK-7727] [REST] Improve error logging in StaticFileServerHandlers

is · Feb 19, 2020 · 20fa7ed · 20fa7ed
1 parent 7cfd898
commit 20fa7ed
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 38 deletions.
diff --git a/...ava/org/apache/flink/runtime/webmonitor/history/HistoryServerStaticFileServerHandler.java b/...ava/org/apache/flink/runtime/webmonitor/history/HistoryServerStaticFileServerHandler.java
@@ -171,25 +171,7 @@ private void respondWithFile(ChannelHandlerContext ctx, HttpRequest request, Str
  }
  }
 
- if (!file.exists() || file.isHidden() || file.isDirectory() || !file.isFile()) {
- HandlerUtils.sendErrorResponse(
- ctx,
- request,
- new ErrorResponseBody("File not found."),
- NOT_FOUND,
- Collections.emptyMap());
- return;
- }
-
- if (!file.getCanonicalFile().toPath().startsWith(rootPath.toPath())) {
- HandlerUtils.sendErrorResponse(
- ctx,
- request,
- new ErrorResponseBody("File not found."),
- NOT_FOUND,
- Collections.emptyMap());
- return;
- }
+ StaticFileServerHandler.checkFileValidity(file, rootPath, ctx, request, Collections.emptyMap(), LOG);
 
  // cache validation
  final String ifModifiedSince = request.headers().get(IF_MODIFIED_SINCE);
@@ -220,6 +202,9 @@ private void respondWithFile(ChannelHandlerContext ctx, HttpRequest request, Str
  try {
  raf = new RandomAccessFile(file, "r");
  } catch (FileNotFoundException e) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Could not find file {}.", file.getAbsolutePath());
+ }
  HandlerUtils.sendErrorResponse(
  ctx,
  request,

diff --git a/...main/java/org/apache/flink/runtime/rest/handler/legacy/files/StaticFileServerHandler.java b/...main/java/org/apache/flink/runtime/rest/handler/legacy/files/StaticFileServerHandler.java
@@ -52,6 +52,8 @@
 import org.apache.flink.shaded.netty4.io.netty.handler.ssl.SslHandler;
 import org.apache.flink.shaded.netty4.io.netty.handler.stream.ChunkedFile;
 
+import org.slf4j.Logger;
+
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -68,6 +70,7 @@
 import java.util.Date;
 import java.util.GregorianCalendar;
 import java.util.Locale;
+import java.util.Map;
 import java.util.TimeZone;
 
 import static org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpHeaders.Names.CACHE_CONTROL;
@@ -77,7 +80,9 @@
 import static org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpHeaders.Names.EXPIRES;
 import static org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpHeaders.Names.IF_MODIFIED_SINCE;
 import static org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpHeaders.Names.LAST_MODIFIED;
+import static org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpResponseStatus.FORBIDDEN;
 import static org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR;
+import static org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpResponseStatus.METHOD_NOT_ALLOWED;
 import static org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND;
 import static org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpResponseStatus.NOT_MODIFIED;
 import static org.apache.flink.shaded.netty4.io.netty.handler.codec.http.HttpResponseStatus.OK;
@@ -194,25 +199,7 @@ private void respondToRequest(ChannelHandlerContext ctx, HttpRequest request, St
  }
  }
 
- if (!file.exists() || file.isHidden() || file.isDirectory() || !file.isFile()) {
- HandlerUtils.sendErrorResponse(
- ctx,
- request,
- new ErrorResponseBody("File not found."),
- NOT_FOUND,
- responseHeaders);
- return;
- }
-
- if (!file.getCanonicalFile().toPath().startsWith(rootPath.toPath())) {
- HandlerUtils.sendErrorResponse(
- ctx,
- request,
- new ErrorResponseBody("File not found."),
- NOT_FOUND,
- responseHeaders);
- return;
- }
+ checkFileValidity(file, rootPath, ctx, request, responseHeaders, logger);
 
  // cache validation
  final String ifModifiedSince = request.headers().get(IF_MODIFIED_SINCE);
@@ -244,6 +231,9 @@ private void respondToRequest(ChannelHandlerContext ctx, HttpRequest request, St
  raf = new RandomAccessFile(file, "r");
  }
  catch (FileNotFoundException e) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Could not find file {}.", file.getAbsolutePath());
+ }
  HandlerUtils.sendErrorResponse(
  ctx,
  request,
@@ -374,4 +364,46 @@ public static void setContentTypeHeader(HttpResponse response, File file) {
  String mimeFinal = mimeType != null ? mimeType : MimeTypes.getDefaultMimeType();
  response.headers().set(CONTENT_TYPE, mimeFinal);
  }
+
+ public static void checkFileValidity(File file, File rootPath, ChannelHandlerContext ctx, HttpRequest request, Map<String, String> responseHeaders, Logger logger) throws IOException {
+ // this check must be done first to prevent probing for arbitrary files
+ if (!file.getCanonicalFile().toPath().startsWith(rootPath.toPath())) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Requested path {} points outside the root directory.", file.getAbsolutePath());
+ }
+ HandlerUtils.sendErrorResponse(
+ ctx,
+ request,
+ new ErrorResponseBody("Forbidden."),
+ FORBIDDEN,
+ responseHeaders);
+ return;
+ }
+
+ if (!file.exists() || file.isHidden()) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Requested path {} cannot be found.", file.getAbsolutePath());
+ }
+ HandlerUtils.sendErrorResponse(
+ ctx,
+ request,
+ new ErrorResponseBody("File not found."),
+ NOT_FOUND,
+ responseHeaders);
+ return;
+ }
+
+ if (file.isDirectory() || !file.isFile()) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Requested path {} does not point to a file.", file.getAbsolutePath());
+ }
+ HandlerUtils.sendErrorResponse(
+ ctx,
+ request,
+ new ErrorResponseBody("File not found."),
+ METHOD_NOT_ALLOWED,
+ responseHeaders);
+ return;
+ }
+ }
 }