Some code issues found by Static Code Analyzer that may need reviewing

LabKey Support Forum
Some code issues found by Static Code Analyzer that may need reviewing stevec  2020-11-09 10:17
Status: Closed
 

I ran a static Code Analyzer on the Labkey code and found the following issues that may need reviewing:

========================================================================================

  1. The String equals method is called with argument org.labkey.api.util.Path, not a String. The equals method will always return false.

2 examples, please note the getPath() method returns a org.labkey.api.util.Path, not a String.

Example 1

org.labkey.api.webdav.AbstractWebdavResource
				.......
				.......
  	  			public boolean canRead(User user, boolean forRead)
    				{
        			if ("/".equals(getPath()))

=====================================================================================
Example 2

			org.labkey.api.webdav.WebFilesResolverImpl
				.......
				.......
				 public boolean canRead(User user, boolean forRead)
				 {
         			 if ("/".equals(getPath()))

~========================================================================================

  1. The org.labkey.api.util.Path equals method is called and passes a String, the org.labkey.api.util.Path.equals method will return false when the input Object is not org.labkey.api.util.Path,

File: ./server/modules/platform/core/src/org/labkey/core/webdav/DavController.java


	      public void writeProperties(WebdavResource resource, Find type, List<String> propertiesVector)

			......
			......
			.......

   	          String displayName = resource.getPath().equals("/") ? "/" : resource.getName();

========================================================================================



 		       public void writeProperties(WebdavResource resource, Find type, List<String> propertiesVector) throws Exception
        		{
            		json.object();
            		json.key("id").value(resource.getPath());
            		String displayName = resource.getPath().equals("/") ? "/" : resource.getName();

========================================================================================
~

  1. The HashMap.get(Object) call passes in object ColumnInfo which is not the key defined for the HashMap. The HashMap key is a String.


     Map<String, Integer> outputMap = DataIteratorUtil.createColumnAndPropertyMap(it);       // String is the key to this HashMap

        ColumnInfo containerColumn = table.getColumn("container");
 

        Integer indexContainer = outputMap.get(containerColumn);       // containerColumn is not a String

=====================================================================================

~
4. NullPointerExceptions

variable raf will get NullPointerException

class: org.labkey.core.webdav.DavbController$PutAction

WebdavStatus doMethod() throws DavException, IOException, RedirectException {

....
.....
....

           RandomAccessFile raf = null;
            OutputStream os = null;

            try
            {
                if (!exists)
                {
                    temp = getTemporary();
                   if (temp)
                        markTempFile(resource);
                    deleteFileOnFail = true;
                }

                File file = resource.getFile();
                boolean isBrowserDev = getUser().isTrustedBrowserDev();
                if (range != null)
                {
                    if (resource.getContentType().startsWith("text/html") && !isBrowserDev)
                        throw new DavException(WebdavStatus.SC_FORBIDDEN, "Partial writing of html files is not allowed");
                    if (null != AntiVirusService.get())
                        throw new DavException(WebdavStatus.SC_FORBIDDEN, "Partial writing not supported with virus scanner enabled");
                    if (range.start > raf.length() || (range.end - range.start) > Integer.MAX_VALUE)

=====================================================================================

variable context will get NullPointerException

org.labkey.api.reports.report.ScriptReportDescriptor

   public ReportDescriptorDocument getDescriptorDocument(ImportContext context)
    {
        // if we are doing folder export (or module file save), we don't want to double save the script property
        if (null != context)
            return getDescriptorDocument(context.getContainer(), context, true, Set.of(Prop.script.name()));
        else
            return getDescriptorDocument(context.getContainer(), context, true, Collections.emptySet());
    }

=====================================================================================

  1. To honor if statement, remove Semicolon at:

if (BACKGROUND_WELL_GROUP.equals(group.getName()));

class: org.labkey.elispot.ElispotPlateTypeHandler

    {
        Map<String, Map<String, Double>> backgroundMap = new HashMap<>();
        WellGroup backgroundGroup = null;

        for (WellGroup group : plate.getWellGroups(WellGroup.Type.CONTROL))
        {
            if (BACKGROUND_WELL_GROUP.equals(group.getName()));
            {
                backgroundGroup = group;
                break;
            }
        }
 
 
Jon (LabKey DevOps) responded:  2020-11-09 11:04
Hello Steve,

Thank you for bringing this to our attention! We will have this reviewed by our development team.

Regards,

Jon