Uploaded image for project: 'Sakai'
  1. Sakai
  2. SAK-17647 Clean up code via static code review sweep for 2.7
  3. SAK-17677

A couple of leaks of InputStream and NPE's found by static code review sweep for PODCASTs

    Details

    • Type: Sub-task
    • Status: CLOSED
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.7.0
    • Fix Version/s: 2.7.0, 2.8.0
    • Component/s: Podcasts
    • Labels:
      None
    • Environment:
      Static code review of 2.7.0-M2

      Description

      [1] podPermBean
      Not used Line 148
      UIColumn column = new UIColumn();

      [2] podPermBean
      Not used Line line 275
      final Collection podcasts = new ArrayList();

      [3] podPermBean
      Not used Line 267
      final int width = headerRow.size();

      [4] podHomeBean
      Line 1251 – Need to close fileAsStream on IOException. Please verify that fileAsStream properly closed later as well.

      try

      { fileAsStream.read(fileContents); }


      catch (IOException e)

      { LOG.error("IOException while attempting the actual upload file " + filename + " during processAdd " + " for site " + podcastService.getSiteId() + ". " + e.getMessage(), e); setErrorMessage(IO_ALERT); // stay on Add podcast page return "podcastAdd"; }

      [5]
      Line 1430 - Need to close fileAsStream on IOException. Please verify that fileAsStream properly closed later as well.

      try

      { fileAsStream.read(fileContents); }
      catch (IOException e) { LOG.error("IOException while attempting to get file contents when revising podcast for " + filename + " in site " + podcastService.getSiteId() + ". " + e.getMessage(), e); setErrorMessage(IO_ALERT); return "podcastRevise"; }

      [6,7,8,9] If you replace fileAsStream with nulls then they may not be closed up until GC. Please review the whole class and close not null (in a try catch finally block)
      Line 1304,1377,1554,1569,
      fileAsStream = null;

      [10] org.sakaiproject.tool.podcasts.podHomeBean
      If fileAsStream == null NPE for fileAsStream.read(fileContents);
      Line 1430

      if (fileAsStream != null) { fileContents = new byte[(int) fileSize]; }
      else { fileContents = new byte[(int) selectedPodcast.fileSize]; }

      try { fileAsStream.read(fileContents); }

      [11] org.sakaiproject.tool.podcasts.jsf.tag.DatePickerTag
      Not used Line 93 -
      FacesContext context = getFacesContext();

      [12] org.sakaiproject.tool.podcasts.jsf.tag.TagUtil
      Not used Line 193
      Application app = context.getApplication();

      [13] TagUtil
      Line 299 etc
      Consider returning false instead of null

      public static Boolean evalBoolean(String expression)
      {
      if (expression == null)

      { return null; }

      if (UIComponentTag.isValueReference(expression))
      {
      FacesContext context = FacesContext.getCurrentInstance();
      Application app = context.getApplication();
      Object r = app.createValueBinding(expression).getValue(context);
      if (r == null)

      { return null; }

      else if (r instanceof Boolean)

      { return (Boolean) r; }

      else

      { return new Boolean(r.toString()); }

      }
      else

      { return new Boolean(expression); }

      }

        Gliffy Diagrams

          Zeplin

            Attachments

              Activity

                People

                Assignee:
                a.m.berg@uva.nl Alan Berg
                Reporter:
                a.m.berg@uva.nl Alan Berg
                Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                  Dates

                  Created:
                  Updated:
                  Resolved:

                    Git Integration