Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NullPointerException when parsing VCalendar #116

Open
Olafxso opened this issue Oct 28, 2019 · 1 comment
Open

NullPointerException when parsing VCalendar #116

Olafxso opened this issue Oct 28, 2019 · 1 comment

Comments

@Olafxso
Copy link
Contributor

Olafxso commented Oct 28, 2019

Hi,

I tried to read a calendar but get an error:

java.lang.NullPointerException
	at jfxtras.icalendarfx.VParentBase.checkChild(VParentBase.java:370)
	at jfxtras.icalendarfx.VParentBase.addChildInternal(VParentBase.java:397)
	at jfxtras.icalendarfx.VParentBase.parseContent(VParentBase.java:305)
	at jfxtras.icalendarfx.VParentBase.parseContent(VParentBase.java:283)
	at jfxtras.icalendarfx.VCalendar.parse(VCalendar.java:956)
	at TestVCalendar.main(TestVCalendar.java:18)

.
Here is my test case:

public class TestVCalendar
{
  public static void main( String[] args )
  {
    try (InputStream fin = new URL(
        "https://api.socialschools.eu/api/v1/icalfeed/?schoolId=1406&roleTypeId=1&userId=8452ab23-41bc-4fe7-bdb2-1a232ebe16ea&hash=B23ERfs2lNttZ17UG78QmPY2mPLFKCVVn6cGYwSEGs" )
            .openStream())
    {
      InputStreamReader reader = new InputStreamReader( fin, StandardCharsets.UTF_8 );
      System.out.println( VCalendar.parse( reader ) );
    }
    catch ( Exception e )
    {
      e.printStackTrace();
    }
  }
}

I also make a workaround in VParentBase.checkChild. The variable getter is null in this case. I have Move the part below if ( !isChildAllowed )to an else block. Now it is ok for me.

Here is the complete method:

   protected boolean checkChild( List<Message> messages, String content, String elementName, VChild newChild )
  {
    int initialMessageSize = messages.size();
    if ( newChild == null )
    {
      Message message = new Message( this,
          "Ignored invalid element:" + content,
          MessageEffect.MESSAGE_ONLY );
      messages.add( message );
    }
    Method getter = getGetter( newChild );
    boolean isChildAllowed = getter != null;
    if ( !isChildAllowed )
    {
      Message message = new Message( this,
          elementName + " not allowed in " + name(),
          MessageEffect.THROW_EXCEPTION );
      messages.add( message );
    }
    else // Moved to an else block, because getter could be null here
    {
      final boolean isChildAlreadyPresent;
      Object currentParameter = null;
      try
      {
        currentParameter = getter.invoke( this );
      }
      catch ( IllegalAccessException | IllegalArgumentException | InvocationTargetException e )
      {
        e.printStackTrace();
      }
      if ( currentParameter instanceof Collection )
      {
        isChildAlreadyPresent = ( (Collection<?>)currentParameter ).contains( newChild ); // TODO contains is expensive - try to find a way to avoid
      }
      else
      {
        isChildAlreadyPresent = currentParameter != null;
      }
      if ( isChildAlreadyPresent )
      {
        Message message = new Message( this,
            newChild.getClass().getSimpleName() + " can only occur once in a calendar component.  Ignoring instances beyond first.",
            MessageEffect.MESSAGE_ONLY );
        messages.add( message );
      }
    }
    return messages.size() == initialMessageSize;
  }

What do you think of this solution?

@daviddbal
Copy link
Member

daviddbal commented Oct 29, 2019

Yes, I think your code change is smart. Would you like to commit it and make a PR?

Olafxso added a commit to Olafxso/jfxtras that referenced this issue Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants