BootNotificationRequest::setChargeBoxSerialNumber(null) throws NullPointerException

[bantu]

I am not sure whether this is the expected behavior, I do not believe it is.

new BootNotificationRequest().setChargeBoxSerialNumber(null);

throws a NullPointerException due to .length() in

    if (!ModelUtil.validate(chargeBoxSerialNumber, STRING_25_CHAR_MAX_LENGTH)) {
      throw new PropertyConstraintException(
          chargeBoxSerialNumber.length(), validationErrorMessage(STRING_25_CHAR_MAX_LENGTH));
    }

being called on a null String.

Note that chargeBoxSerialNumber is optional in BootNotificationRequest.

Here's a test for reproduction:

package eu.chargetime.ocpp.model.core.test;

import eu.chargetime.ocpp.model.core.BootNotificationRequest;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

public class BootNotificationRequestTest {

  @Rule public ExpectedException thrownException = ExpectedException.none();

  private BootNotificationRequest request;

  @Before
  public void setup() {
    request = new BootNotificationRequest();
  }

  @Test()
  public void validate_setChargeBoxSerialNumber_onNull_doesNotThrowNPE() {
    request.setChargeBoxSerialNumber(null);
  }
}

[bantu]

Created a branch with the test here: https://github.com/ChargeTimeEU/Java-OCA-OCPP/compare/master...bantu:Java-OCA-OCPP:test-for-github-221?expand=1

[TVolden]

Hi @bantu,

Your right, that makes no sense. Thanks for pointing it out.
Maybe the ModelUtil should have a small method to extract lengths or 0 from a string?

If you make a PR, I'll merge it ;)

- Thomas

[jmluy]

This also affects the other optional fields in that class like chargePointSerial;Number, firmwareVersion, iccid, imsi, meterSerialNumber and meterType.

[bantu]

This also affects the other optional fields in that class like chargePointSerial;Number, firmwareVersion, iccid, imsi, meterSerialNumber and meterType.

Yes. Thank you. I was about to mention that. Does it also affect other classes?

[jmluy]

This also affects the other optional fields in that class like chargePointSerial;Number, firmwareVersion, iccid, imsi, meterSerialNumber and meterType.

Yes. Thank you. I was about to mention that. Does it also affect other classes?

I didn't check all but for some classes they do a check with something like the following.
if (field != null && ...)

I saw some code which probably have the same issue

• Java-OCA-OCPP/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/model/core/DataTransferRequest.java

  Lines 110 to 116 in 1c53b53

  	public void setMessageId(String messageId) {
  	if (!isValidMessageId(messageId)) {
  	throw new PropertyConstraintException(messageId.length(), createErrorMessage(50));
  	}
  	this.messageId = messageId;
  	}

• Java-OCA-OCPP/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/model/core/GetConfigurationRequest.java

  Lines 65 to 71 in 1c53b53

  	private void validateKeys(String[] keys) {
  	for (String k : keys) {
  	if (!ModelUtil.validate(k, 50)) {
  	throw new PropertyConstraintException(k.length(), "Exceeds limit of 50 chars");
  	}
  	}
  	}

[bantu]

@jmluy I'll focus on BootNotificationRequest.

[bantu]

See #223