Re: [Issue 3770] New - JavaHL method to set binary property is broken
On Wed, Dec 29, 2010 at 4:43 PM, markp...@tigris.org wrote:
http://subversion.tigris.org/issues/show_bug.cgi?id=3770
Issue #|3770
Summary|JavaHL method to set binary property is broken
Component|subversion
Version|1.6.x
Platform|Macintosh
URL|
OS/Version|All
Status|NEW
Status whiteboard|
Keywords|
Resolution|
Issue type|DEFECT
Priority|P2
Subcomponent|bindings_javahl
Assigned to|iss...@subversion
Reported by|markphip
--- Additional comments from markp...@tigris.org Wed Dec 29 13:43:28
-0800 2010 ---
JavaHL has a method to set a property using a byte array. This method
existed so that binary
properties, such as an image thumbnail can be set on a file. If you go back
to the source for SVN 1.2,
this went direct to a native method. Somewhere along the way (probably in
1.5 release) as new method
signatures were added and we sought to reuse those methods we broke this
function. Look at the
current SVN 1.6.x implementation:
/**
* @deprecated Use {...@link #propertySet(String, String, String, int,
* String[], boolean, Map)} instead.
* @since 1.2
*/
public void propertySet(String path, String name, byte[] value,
boolean recurse, boolean force)
throws ClientException
{
propertySet(path, name, new String(value), recurse, force);
}
The incoming byte[] is converted to a String and the common method for using
a String is used. This
causes binary values to be corrupted. I think we need to add a native method
back to the JavaHL library
which receives a pure byte array.
Mark,
Daniel pointed out on IRC that all the revpropTable arguments in the
JavaHL API are MapString, String. Should they be adjusted to
MapString, byte[] ?
-Hyrum
Re: [Issue 3770] New - JavaHL method to set binary property is broken
On Thu, Dec 30, 2010 at 2:17 PM, Hyrum K Wright hy...@hyrumwright.org wrote: Mark, Daniel pointed out on IRC that all the revpropTable arguments in the JavaHL API are MapString, String. Should they be adjusted to MapString, byte[] ? What is the rule for revision properties? I thought they had to be UTF-8 strings, in which case the Java String class seems more appropriate than using Byte[] which would imply the user can assign binary values to the property. If Revision properties can contain binary values, then yes we should not be using String here. -- Thanks Mark Phippard http://markphip.blogspot.com/
Re: [Issue 3770] New - JavaHL method to set binary property is broken
On Thu, Dec 30, 2010 at 2:57 PM, Mark Phippard markp...@gmail.com wrote: On Thu, Dec 30, 2010 at 2:17 PM, Hyrum K Wright hy...@hyrumwright.org wrote: Mark, Daniel pointed out on IRC that all the revpropTable arguments in the JavaHL API are MapString, String. Should they be adjusted to MapString, byte[] ? What is the rule for revision properties? I thought they had to be UTF-8 strings, in which case the Java String class seems more appropriate than using Byte[] which would imply the user can assign binary values to the property. If Revision properties can contain binary values, then yes we should not be using String here. I don't remember what the repository enforces, but the underlying API contains a hash of const char * revprop names, with values of svn_string_t *, which is our counted string which can contain arbitrary binary data. So the client API allows binary data, but it might be caught further down the library stack. -Hyrum
Re: [Issue 3770] New - JavaHL method to set binary property is broken
Hyrum K Wright wrote on Thu, Dec 30, 2010 at 15:03:04 -0500: On Thu, Dec 30, 2010 at 2:57 PM, Mark Phippard markp...@gmail.com wrote: On Thu, Dec 30, 2010 at 2:17 PM, Hyrum K Wright hy...@hyrumwright.org wrote: Mark, Daniel pointed out on IRC that all the revpropTable arguments in the JavaHL API are MapString, String. Should they be adjusted to MapString, byte[] ? What is the rule for revision properties? I thought they had to be UTF-8 strings, in which case the Java String class seems more appropriate than using Byte[] which would imply the user can assign binary values to the property. If Revision properties can contain binary values, then yes we should not be using String here. I don't remember what the repository enforces, but the underlying API contains a hash of const char * revprop names, with values of svn_string_t *, which is our counted string which can contain arbitrary binary data. So the client API allows binary data, but it might be caught further down the library stack. % $svn ps --revprop -r0 svn-binary -F =svn wc1 property 'svn-binary' set on repository revision 0 % $svn ps --revprop -r0 random-data -F =(head -c 1024 /dev/urandom) wc1 property 'random-data' set on repository revision 0 % $svn pg --revprop -r0 random-data --strict wc1 | xxd -ps -c1 | grep 00 00 00 00 -Hyrum
