Incorrect handling of a null return value

• Key: TRB-8
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Scott Eade
• Reporter: Gunther Olesch
• Component/s: Core
• Affects Version/s: Core 2.3.2

Description

I have found a bug within turbine 2.3.2 which results in a NullPointer to be thrown within TemplateURI Line 274.

I found the Bug because I used $link.addPathInfo($data.getParameters()) which takes all keys within the request, and adds the Keys and their values to the link. The Problem is within getStrings(String key) which returns null of no values are found. TemplateURI does not check the return value for null. I have a patch ready.

Comments

[Gunther Olesch]

a patch fixing the issue.

[Gunther Olesch]

Found another location

[Gunther Olesch]

I have digged a little deeper into the problem as it seemed to me that there should never be a return value of null in the first place. Here is what I came up with:

Here is the Exception again
org.apache.turbine.services.pull.tools.TemplateLink.addPathInfo threw Exception java.lang.NullPointerException at org.apache.turbine.util.uri.TemplateURI.add(TemplateURI.java:274)

So looking in TemplateURI's add Method, the first thing done is to call keySet() on the ParameterParser to retrieve all the Keys within the request. Then the Method getStrings is called for each Key within the KeySet on the ParameterParser Object. The ParameterParser is most certainly an Instance of the DefaultParameterParser which inherits from BaseValueParser.

If we do now have a form submission which contains an input field of type file, we have a problem because the ParameterParser maintains two maps internally one for fileItems and one for the other parameters. But keySet returns all the Keys that are within the request by returning a CompositeSet with the keys from both maps. BUT the Method getStrings just checks the Map without the fileItems. That is why the Key is never found, and null is returned.

Then I compared t2.3.2's source with t2.3.1's source and found that the Method keySet() there did not return a CompositeSet of the fileItems and the rest, but only the restKeys.

I hope i could get the problem across...

[Scott Eade]

I added a coupld of test cases to TurbineURITest in the 2.3 branch to try and catch this problem but my tests pass.

Under what circumstances would pp.getStrings(key) return null?

[Jürgen Hoffmann]

I have added a method to the DefaultParameterParserTest, which addresses this issues. As you can see it falis ;) and does the same as does TemplateURI in Line 273

HTH

Buddy

[Scott Eade]

The test case you have added seems to be for an unrelated ussue (take a look at TRB-10).

Scott

[Jürgen Hoffmann]

well the Problem I am adressing is the fact that the DefaultParameterParser provides the function keySet() which looks like

    public Set keySet()
    {
        return new CompositeSet(new Set[] { super.keySet(), fileParameters.keySet() } );
    }

as you can see this returns a CompositeSet made up of 2 seperate Sets, TemplateURI makes use of this function within

    protected void add(int type, ParameterParser pp)
    {
        for(Iterator it = pp.keySet().iterator(); it.hasNext();) <<----- here is the call
        {
            String key = (String) it.next();
            
            if (!key.equalsIgnoreCase(CGI_ACTION_PARAM) &&
                    !key.equalsIgnoreCase(CGI_SCREEN_PARAM) &&
                    !key.equalsIgnoreCase(CGI_TEMPLATE_PARAM))
            {
                String[] values = pp.getStrings(key); <---- this is causing the problem though
                if(values != null)
                {
                    for (int i = 0; i < values.length; i++)
                    {
                        add(type, key, values[i]);
                    }
                }
                else
                {
                    add(type, key, "");
                }
            }
        }
    }

getStrings is a Method defined in the BaseValueParser which does not know about the fileParameters Map. get Strings calls

    public String[] getStrings(String name)
    {
        return getParam(name);
    }

which calls

    protected String [] getParam(final String name)
    {
        String key = convert(name);

        return (key != null) ? (String []) parameters.get(key) : null;
    }

which does as you see not know about fileParameters Map.

So in my eyes as long as DefaultParameterParser.KeySet() returns the keys of Both Maps, one would get a NPE if he is using TemplateURI.add(Integer, ParameterParser) if he has uploaded a File before.

Hopefully this explanation shows the problem and why I think the problem is at least to this extend a Prolem within TemplateURI

[Scott Eade]

Patch committed.

Thanks.

[Thomas Vandahl]

Fix is in 2.3.3-release