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 wrote: > > On Thu, Dec 30, 2010 at 2:17 PM, Hyrum K Wright > > wrote: > > > >> Mark, > >> Daniel pointed out on IRC that all the revpropTable arguments in the > >> JavaHL API are Map. Should they be adjusted to > >> Map ? > > > > 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
Re: [Issue 3770] New - JavaHL method to set binary property is broken
On Thu, Dec 30, 2010 at 2:57 PM, Mark Phippard wrote: > On Thu, Dec 30, 2010 at 2:17 PM, Hyrum K Wright wrote: > >> Mark, >> Daniel pointed out on IRC that all the revpropTable arguments in the >> JavaHL API are Map. Should they be adjusted to >> Map ? > > 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
On Thu, Dec 30, 2010 at 2:17 PM, Hyrum K Wright wrote: > Mark, > Daniel pointed out on IRC that all the revpropTable arguments in the > JavaHL API are Map. Should they be adjusted to > Map ? 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 Wed, Dec 29, 2010 at 4:43 PM, 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 Map. Should they be adjusted to Map ? -Hyrum