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

Input buffer size value gets truncated when it exceeds the size limit #263

Open
Sakthi-SM opened this issue Apr 4, 2023 · 3 comments
Open

Comments

@Sakthi-SM
Copy link
Contributor

If the input buffer size is assigned a value greater than the size of int32, then the value is truncated and assigned to the buffer size.

I tried to assign a value of 800000000000 to the buffer size and the expectation was that it would raise an error. Rather the buffer was assigned with a value of 1136082944.
NI IO Trace shows that the input_buf_size is getting truncated from 800000000000 to 1136082944 before calling the DAQmx C API,
image

@WayneDroid
Copy link
Collaborator

The value (greater than int32) passed down to set_buffer_attribute_uint32 and casted to ctypes.c_uint32 causing the value got truncated. Picture below is showing 800000000000 (39 bits) truncated to 1136082944 after setting 0 to 33rd bit to 39th bit.

Suggest validating the value if it is within range in input_buf_size.setter and output_buf_size.setter. Raise error if value is out of range.

image

@bkeryan bkeryan removed their assignment Feb 22, 2024
@bkeryan
Copy link
Collaborator

bkeryan commented Feb 22, 2024

@WayneDroid This issue is not unique to the buffer size attributes. If ctypes doesn't perform a range check when we convert an int to a ctypes sized integer type, then I think this affects every API function that has integer parameters.

Protobuf does perform a range check when assigning integer fields, so GrpcStubInterpreter raises a ValueError in this situation. I think it would be reasonable to fix this issue by adding auto-generated range checks to LibraryInterpreter.

Here is the test output if I update it to demonstrate this issue:

tests/component/_task_modules/test_in_stream_buffer_properties.py::test___ai_task___set_int32_property___value_is_set[library_init_kwargs] FAILED [ 50%]
tests/component/_task_modules/test_in_stream_buffer_properties.py::test___ai_task___set_int32_property___value_is_set[grpc_init_kwargs] FAILED [100%]

====================================================== FAILURES =======================================================
_______________________ test___ai_task___set_int32_property___value_is_set[library_init_kwargs] _______________________

task = Task(name=_unnamedTask<0>), sim_6363_device = Device(name=nidaqmxMultithreadingTester1)

    def test___ai_task___set_int32_property___value_is_set(task, sim_6363_device):
        task.ai_channels.add_ai_voltage_chan(sim_6363_device.ai_physical_chans[0].name)
        task.timing.samp_timing_type = SampleTimingType.SAMPLE_CLOCK

        # Setting a valid input buffer size of type int32
        task.in_stream.input_buf_size = 800000000000

>       assert task.in_stream.input_buf_size == 800000000000
E       assert 1136082944 == 800000000000
E        +  where 1136082944 = InStream(task=_unnamedTask<0>).input_buf_size
E        +    where InStream(task=_unnamedTask<0>) = Task(name=_unnamedTask<0>).in_stream

tests\component\_task_modules\test_in_stream_buffer_properties.py:11: AssertionError
________________________ test___ai_task___set_int32_property___value_is_set[grpc_init_kwargs] _________________________

task = Task(name=_unnamedTask<0>), sim_6363_device = Device(name=nidaqmxMultithreadingTester1)

    def test___ai_task___set_int32_property___value_is_set(task, sim_6363_device):
        task.ai_channels.add_ai_voltage_chan(sim_6363_device.ai_physical_chans[0].name)
        task.timing.samp_timing_type = SampleTimingType.SAMPLE_CLOCK

        # Setting a valid input buffer size of type int32
>       task.in_stream.input_buf_size = 800000000000

tests\component\_task_modules\test_in_stream_buffer_properties.py:9:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
generated\nidaqmx\_task_modules\in_stream.py:276: in input_buf_size
    self._interpreter.set_buffer_attribute_uint32(self._handle, 0x186c, val)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <nidaqmx._grpc_interpreter.GrpcStubInterpreter object at 0x000002205D9F3C40>, task = name: "ni:generated:1"

attribute = 6252, value = 800000000000

    def set_buffer_attribute_uint32(self, task, attribute, value):
        response = self._invoke(
            self._client.SetBufferAttributeUInt32,
>           grpc_types.SetBufferAttributeUInt32Request(
                task=task, attribute_raw=attribute, value=value))
E       ValueError: Value out of range: 800000000000

generated\nidaqmx\_grpc_interpreter.py:2904: ValueError
=============================================== short test summary info ===============================================
FAILED tests/component/_task_modules/test_in_stream_buffer_properties.py::test___ai_task___set_int32_property___value_is_set[library_init_kwargs] - assert 1136082944 == 800000000000
FAILED tests/component/_task_modules/test_in_stream_buffer_properties.py::test___ai_task___set_int32_property___value_is_set[grpc_init_kwargs] - ValueError: Value out of range: 800000000000
========================================= 2 failed, 1740 deselected in 2.79s ==========================================

@bkeryan
Copy link
Collaborator

bkeryan commented Feb 22, 2024

@WayneDroid Two more things:

We should figure out whether this issue is specific to the attribute setter functions. The LibraryInterpreter attribute get/set functions are special because they use varargs. Instead of specifying the ctypes value type (e.g. ctypes.c_uint) in the argtypes array, these functions cast the value to the ctypes value type. This is because the same argtypes array is used for multiple data types. It is conceivable that ctypes might do a range check when we specify the type in the argtypes array. Try it and see.

Also, the Pydantic module may be able to do this sort of range validation based on type hints. However, I haven't used it before and I'm aware that it recently went through a large compatibility break.

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

3 participants