A small bug in the atomicwritefilerequest

[robert bouwens]

basically it works.
but the standard says exactly when a bacneterrorexception has to be thrown:

• negative start
• start exceeds the filesize

at the same time a few internal 'things' should be checked. when the device property backupAndRestoreState
is set to preparingForBackup or preparingForRestore the the atomic read/write requests are dangerous and could deliver
faulty results. thus throw a device configuration in progress bacnetserviceexception as written in the standard.

	@Override
	public AcknowledgementService handle(LocalDevice localDevice, Address from, Network network) throws BACnetException
	{
		AtomicReadFileAck response;

		BACnetObject obj;
		FileObject file;
		try
		{
			// Find the file.
			obj = localDevice.getObjectRequired(fileIdentifier);
			if (!(obj instanceof FileObject))
			{
				System.out.println("File access request on an object that is not a file");
				throw new BACnetServiceException(ErrorClass.object, ErrorCode.rejectInconsistentParameters);
			}
			// check for status (backup/restore)
			BackupState bsOld = (BackupState) localDevice.getConfiguration().getProperty(
					PropertyIdentifier.backupAndRestoreState);
			if (bsOld.intValue() == BackupState.preparingForBackup.intValue()
					|| bsOld.intValue() == BackupState.preparingForRestore.intValue())
			{
				// send error: device configuration in progress as response
				throw new BACnetServiceException(ErrorClass.device, ErrorCode.configurationInProgress);
			}

			file = (FileObject) obj;

			// Validation.
			FileAccessMethod fileAccessMethod = (FileAccessMethod) file.getProperty(PropertyIdentifier.fileAccessMethod);
			if (recordAccess && fileAccessMethod.equals(FileAccessMethod.streamAccess) || !recordAccess
					&& fileAccessMethod.equals(FileAccessMethod.recordAccess))
				throw new BACnetErrorException(getChoiceId(), ErrorClass.object, ErrorCode.invalidFileAccessMethod);
		}
		catch (BACnetServiceException e)
		{
			throw new BACnetErrorException(getChoiceId(), e);
		}

		if (recordAccess)
			throw new NotImplementedException();

		long start = fileStartPosition.longValue();
		long length = requestedCount.longValue();

		/* 
		 * throw an exception when the following conditions are met:
		 * - start is a negative number
		 * - start exceeds the length of the file object
		 */
		if (start > file.length() || start < 0)
			throw new BACnetErrorException(getChoiceId(), ErrorClass.object, ErrorCode.invalidFileStartPosition);

		try
		{
			response = new AtomicReadFileAck(new Boolean(file.length() <= start + length), fileStartPosition, file.readData(
					start, length));
		}
		catch (IOException e)
		{
			throw new BACnetErrorException(getChoiceId(), ErrorClass.object, ErrorCode.fileAccessDenied);
		}

		return response;
	}

that's it - works nicely.
robert

[mlohbihler]

What's "recordAccess"? It isn't defined anywhere.

[robert bouwens]

hi matt,
sorry for being unclear:

public class AtomicReadFileRequest extends ConfirmedRequestService
{
	private static final long serialVersionUID = -279843621668191530L;

	public static final byte TYPE_ID = 6;

	private final ObjectIdentifier fileIdentifier;
	private final boolean recordAccess;
	private final SignedInteger fileStartPosition;
	private final UnsignedInteger requestedCount;

	public AtomicReadFileRequest(ObjectIdentifier fileIdentifier, boolean recordAccess, SignedInteger fileStartPosition,
			UnsignedInteger requestedCount)
	{
		this.fileIdentifier = fileIdentifier;
		this.recordAccess = recordAccess;
		this.fileStartPosition = fileStartPosition;
		this.requestedCount = requestedCount;
	}

the varaible is the standard member variable defined in the class.

regards
robert

[mlohbihler]

Ah, that explains it. The title of the post threw me off.

[mlohbihler]

Should the fileStartPosition condition not be:

        if (start < 0 || start >= file.length())

?

Or do you intend that if start == file.length() that a 0 result should be returned?

[robert bouwens]

hello matt,

a zero length file should be readable without error.
and the standard says explicitly when the file size exceeds.

if (start < 0 || start > file.length()

is correct when reading the spec.
and another commercial stack shows the same behaviour with the upper mod.

regards
robert

[mlohbihler]

Ok. I recall (but at the moment can't confirm) that there was some problem with this during testing of the original implementation, but i'll go with your version.