From 2a67d2ba6ffb9e3d39c1f7c6194c6786fe9f5d12 Mon Sep 17 00:00:00 2001 From: luc Date: Tue, 1 Dec 2015 01:06:01 +0100 Subject: [PATCH 1/2] Corrected error management for unsupported image formats, parsing errors, and unavailable resources : avoid logging to much Exceptions as these errors easily occur when searching images. --- htroot/ViewImage.java | 53 +++++++++++-------- .../http/servlets/YaCyDefaultServlet.java | 12 +++++ 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/htroot/ViewImage.java b/htroot/ViewImage.java index d6838dc8b..08d0dd341 100644 --- a/htroot/ViewImage.java +++ b/htroot/ViewImage.java @@ -121,26 +121,30 @@ public class ViewImage { encodedImage = new EncodedImage(image, ext, post.getBoolean("isStatic")); } else { - String urlExt = MultiProtocolURL.getFileExtension(url.getFileName()); - if (ext != null && ext.equalsIgnoreCase(urlExt) && isBrowserRendered(urlExt)) { - return openInputStream(post, sb.loader, auth, url); - } - ImageInputStream imageInStream = null; InputStream inStream = null; - /* - * When opening a file, the most efficient is to open - * ImageInputStream directly on file - */ - if (url.isFile()) { - imageInStream = ImageIO.createImageInputStream(url.getFSFile()); - } else { - inStream = openInputStream(post, sb.loader, auth, url); - imageInStream = ImageIO.createImageInputStream(inStream); - } try { + String urlExt = MultiProtocolURL.getFileExtension(url.getFileName()); + if (ext != null && ext.equalsIgnoreCase(urlExt) && isBrowserRendered(urlExt)) { + return openInputStream(post, sb.loader, auth, url); + } + /* + * When opening a file, the most efficient is to open + * ImageInputStream directly on file + */ + if (url.isFile()) { + imageInStream = ImageIO.createImageInputStream(url.getFSFile()); + } else { + inStream = openInputStream(post, sb.loader, auth, url); + imageInStream = ImageIO.createImageInputStream(inStream); + } // read image encodedImage = parseAndScale(post, auth, urlString, ext, imageInStream); + } catch(Exception e) { + /* Exceptions are not propagated here : many error causes are possible, network errors, + * incorrect or unsupported format, bad ImageIO plugin... + * Instead return an empty EncodedImage. Caller is responsible for handling this correctly */ + encodedImage = new EncodedImage(new byte[0], ext, true); } finally { /* * imageInStream.close() method doesn't close source input @@ -172,7 +176,7 @@ public class ViewImage { * image url. * @return an open input stream instance (don't forget to close it). * @throws IOException - * when a read/write error occured. + * when a read/write error occured. */ private static InputStream openInputStream(final serverObjects post, final LoaderDispatcher loader, final boolean auth, DigestURL url) throws IOException { @@ -185,7 +189,8 @@ public class ViewImage { inStream = loader.openInputStream(loader.request(url, false, true), CacheStrategy.IFEXIST, BlacklistType.SEARCH, agent); } catch (final IOException e) { - ConcurrentLog.fine("ViewImage", "cannot load: " + e.getMessage()); + /** No need to log full stack trace (in most cases resource is not available because of a network error) */ + ConcurrentLog.fine("ViewImage", "cannot load image. URL : " + url); throw e; } } @@ -230,11 +235,11 @@ public class ViewImage { * open stream on image content. Must not be null. * @return an EncodedImage instance. * @throws IOException - * when image could not be parsed or encoded to specified format + * when image could not be parsed or encoded to specified format. */ protected static EncodedImage parseAndScale(serverObjects post, boolean auth, String urlString, String ext, ImageInputStream imageInStream) throws IOException { - EncodedImage encodedImage = null; + EncodedImage encodedImage; // BufferedImage image = ImageIO.read(imageInStream); Iterator readers = ImageIO.getImageReaders(imageInStream); @@ -244,11 +249,13 @@ public class ViewImage { imageInStream.close(); } catch (IOException ignoredException) { } + String errorMessage = "Image format (" + ext + ") is not supported."; + ConcurrentLog.fine("ViewImage", errorMessage + "Image URL : " + urlString); /* * Throw an exception, wich will end in a HTTP 500 response, better * handled by browsers than an empty image */ - throw new IOException("Image format is not supported."); + throw new IOException(errorMessage); } ImageReader reader = readers.next(); reader.setInput(imageInStream, true, true); @@ -319,11 +326,13 @@ public class ViewImage { } else { /* * An error can still occur when transcoding from buffered image to - * target ext : in that case return null + * target ext : in that case EncodedImage.getImage() is empty. */ encodedImage = new EncodedImage(image, ext, isStatic); if (encodedImage.getImage().length() == 0) { - throw new IOException("Image could not be encoded to format : " + ext); + String errorMessage = "Image could not be encoded to format : " + ext; + ConcurrentLog.fine("ViewImage", errorMessage + ". Image URL : " + urlString); + throw new IOException(errorMessage); } } diff --git a/source/net/yacy/http/servlets/YaCyDefaultServlet.java b/source/net/yacy/http/servlets/YaCyDefaultServlet.java index 5ad95e9d0..1606b6650 100644 --- a/source/net/yacy/http/servlets/YaCyDefaultServlet.java +++ b/source/net/yacy/http/servlets/YaCyDefaultServlet.java @@ -851,6 +851,13 @@ public class YaCyDefaultServlet extends HttpServlet { } else if (tmp instanceof EncodedImage) { final EncodedImage yp = (EncodedImage) tmp; result = yp.getImage(); + /** When encodedImage is empty, return a code 500 rather than only an empty response + * as it is better handled across different browsers */ + if(result == null || result.length() == 0) { + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + result.close(); + return; + } if (yp.isStatic()) { response.setDateHeader(HeaderFramework.EXPIRES, now + 600000); // expires in ten minutes } @@ -1004,6 +1011,11 @@ public class YaCyDefaultServlet extends HttpServlet { size += l; } response.setContentLength(size); + } catch(IOException e){ + /** No need to log full stack trace (in most cases resource is not available because of a network error) */ + ConcurrentLog.fine("FILEHANDLER", "YaCyDefaultServlet: resource content stream could not be written to response."); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return; } finally { try { inStream.close(); From 29585e2c5b0cbfcbcc419a46a68b38787687f293 Mon Sep 17 00:00:00 2001 From: luc Date: Tue, 1 Dec 2015 09:55:47 +0100 Subject: [PATCH 2/2] Corrected return type when licence is gone to be consistent with other error cases. --- htroot/ViewImage.java | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/htroot/ViewImage.java b/htroot/ViewImage.java index 461f6787e..249c09432 100644 --- a/htroot/ViewImage.java +++ b/htroot/ViewImage.java @@ -76,10 +76,11 @@ public class ViewImage { * @param env * environment * @return an {@link EncodedImage} instance encoded in format specified in - * post, or an InputStream pointing to original image data + * post, or an InputStream pointing to original image data. + * Return and EncodedImage with empty data when image format is not supported, + * a read/write or any other error occured while loading resource. * @throws IOException - * when specified url is malformed, or a read/write error - * occured, or input or target image format is not supported. + * when specified url is malformed. * Sould end in a HTTP 500 error whose processing is more * consistent across browsers than a response with zero content * bytes. @@ -107,13 +108,15 @@ public class ViewImage { } if ((url == null) && (urlLicense.length() > 0)) { - urlString = URLLicense.releaseLicense(urlLicense); - if (urlString != null) { - url = new DigestURL(urlString); - } else { // license is gone (e.g. released/remove in prev calls) - ConcurrentLog.fine("ViewImage", "image urlLicense not found key=" + urlLicense); - return null; //TODO: maybe favicon accessed again, check iconcache - } + urlString = URLLicense.releaseLicense(urlLicense); + if (urlString != null) { + url = new DigestURL(urlString); + } else { // license is gone (e.g. released/remove in prev calls) + ConcurrentLog.fine("ViewImage", "image urlLicense not found key=" + urlLicense); + /* Return an empty EncodedImage. Caller is responsible for handling this correctly (500 status code response) */ + return new EncodedImage(new byte[0], ext, post.getBoolean("isStatic")); // TODO: maybe favicon accessed again, check + // iconcache + } } // get the image as stream @@ -148,8 +151,8 @@ public class ViewImage { } catch(Exception e) { /* Exceptions are not propagated here : many error causes are possible, network errors, * incorrect or unsupported format, bad ImageIO plugin... - * Instead return an empty EncodedImage. Caller is responsible for handling this correctly */ - encodedImage = new EncodedImage(new byte[0], ext, true); + * Instead return an empty EncodedImage. Caller is responsible for handling this correctly (500 status code response) */ + encodedImage = new EncodedImage(new byte[0], ext, post.getBoolean("isStatic")); } finally { /* * imageInStream.close() method doesn't close source input