Bug #1472

rst::vision:Image.set_data (char* data) truncates on \0 byte

Added by M. Schöpfer about 11 years ago. Updated about 11 years ago.

Status:RejectedStart date:04/22/2013
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:cpp
Target version:-

Description

When you pass an abritatry chunk of data to an rst::vision::Image via set_data (char*), it will get truncated at the first location of \0, as it assumes a c string is passed. (implementation does something like new std::string s, s.assign(data). The .set_data (void*, size_t length) implementation works.

History

#1 Updated by J. Moringen about 11 years ago

First of all, I'm afraid, we cannot do anything about this: The Image.data field is defined as

required bytes data = 1;

in the IDL file and the getter and setter methods (including set_data(char*)) are generated by protoc, the Google protocol buffer compiler, from this definition. We have no direct control over the generated code.

Despite this, what would the correct behavior be? How should a method with signature set_data(char*) determine the size of the blob?

#2 Updated by J. Moringen about 11 years ago

  • Status changed from New to Feedback

#3 Updated by M. Schöpfer about 11 years ago

Ok, I know, this has to be done in order, but when you set width, height and depth of an image, you assume that data should be depth*height*width.

I understand that the code is generated. I do not know if one can do anything about it. If there is nothing one can do about it, maybe documenting the behavior would be a good start so someone else might save the time I spend on finding this bug.

Thanks!

#4 Updated by J. Wienke about 11 years ago

There is a signature with char*, size_t. That's the one you should use.

I don't even see a way how to document this, because this is a specificity for C++ and the code is generated also for Java and Python. Additionally, we would have to add the same comment for every field that uses a bytes as the protocol buffers type.

#6 Updated by J. Moringen about 11 years ago

  • Status changed from Feedback to Rejected

Now I understand. You are right, from a user's perspective the current behavior is far from optimal.

I do not know if one can do anything about it.

If we switch from Google protocol buffers to Rosetta Stone Project at some point, we will be able to enforce constraints like this one:

http://docs.cor-lab.de/rst-manual/trunk/html/generated/stable/package-rst-vision.html#rst.vision.Image.data

We will probably also stop using std::string/char* for binary blobs.

If there is nothing one can do about it, maybe documenting the behavior would be a good start so someone else might save the time I spend on finding this bug.

I'm afraid, we can only refer to the protocol buffer documentation. However, the approach mentioned above will hopefully reduce the potential for such bugs.

#7 Updated by M. Schöpfer about 11 years ago

I know now (as I have written above) that the char*, size_t version works.

So, nevermind, I will just close the issue.

#8 Updated by M. Schöpfer about 11 years ago

-.- someone may close the issue, i cannot

#9 Updated by J. Moringen about 11 years ago

I closed it by changing the status to "Rejected". You should have been able to close the issue as well. Did you maybe try to close concurrently with my update?

#10 Updated by J. Wienke about 11 years ago

There may be different workflow permissions for roles like reporters and developers.

#11 Updated by M. Schöpfer about 11 years ago

Rejected is fine.

I cannot change anything, since I do not have any role whatsoever for this project. Thats fine also ;)

Thanks for your time!

Also available in: Atom PDF