Uploaded image for project: 'Magnolia'
  1. Magnolia
  2. MAGNOLIA-6697

Improve log behavior/levels of: RenderingFilter.getNodedataAsStream as any 404 request will trigger a full stacktrace into the log

XMLWordPrintable

    • Icon: Improvement Improvement
    • Resolution: Unresolved
    • Icon: Neutral Neutral
    • None
    • 5.3.14, 5.4.7
    • rendering

      When requesting an non existing page, trying to fetch the non existing content will trigger a JCR repository exception.
      Clear, as the node does not exist.

      The thrown exception is catched in the end in:
      info.magnolia.rendering.engine.RenderingFilter.getNodedataAsStream(String, Session, HttpServletResponse) line 276

      catch (RepositoryException e) {
                  log.error("RepositoryException while reading Resource [{}]", path, e);
              }
      

      The problem is, that the log level is error.
      This means, on any 404 the logs gets a full stack trace.
      And as bot attacks often search vulnerabilities by calling patterns of expected pages some clients have a huge amount of these log statements in the log. Which makes them grow rapidly in short time.

      The only option the clinet currently have is switching of the logging of that class at all.
      But then the two errors in line 154 & 158 hiden:

      catch (RenderException e) {
                  // TODO better handling of rendering exception
                  // TODO dlipp: why not move this section up to the actual call to render() -> that's the only place where a RenderException could occur...
                  log.error(e.getMessage(), e);
                  throw new ServletException(e);
              } catch (Exception e) {
                  // TODO dlipp: there's no other checked exceptions thrown in the code above - is it correct to react like that???
                  log.error(e.getMessage(), e);
                  if (!response.isCommitted()) {
                      response.setContentType("text/html");
                  }
                  throw new RuntimeException(e);
              }
      

      Pleas investigate into two directions:
      1. Would the log level warn not be more accurate for this situation?
      As it is an expected error when an item is not found in a 404 process.
      2. As the minor solution/workaround:
      Setting the method info.magnolia.rendering.engine.RenderingFilter.getNodedataAsStream from private to protected. So the customer could create a extended custom RenderingFilter and only override this one method to define a different log level.

        Acceptance criteria

              Unassigned Unassigned
              cringele Christian Ringele
              Votes:
              2 Vote for this issue
              Watchers:
              3 Start watching this issue

                Created:
                Updated:

                  Task DoD