You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Hyrum K Wright <hy...@hyrumwright.org> on 2010/12/30 19:17:08 UTC
Re: [Issue 3770] New - JavaHL method to set binary property is broken
On Wed, Dec 29, 2010 at 4:43 PM, <ma...@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|issues@subversion
> Reported by|markphip
>
>
>
>
>
>
> ------- Additional comments from markphip@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<String, String>. Should they be adjusted to
Map<String, byte[]> ?
-Hyrum
Re: [Issue 3770] New - JavaHL method to set binary property is
broken
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
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 <ma...@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 Map<String, String>. Should they be adjusted to
> >> Map<String, 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
Re: [Issue 3770] New - JavaHL method to set binary property is
broken
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
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 <ma...@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 Map<String, String>. Should they be adjusted to
> >> Map<String, 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
Re: [Issue 3770] New - JavaHL method to set binary property is broken
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Thu, Dec 30, 2010 at 2:57 PM, Mark Phippard <ma...@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 Map<String, String>. Should they be adjusted to
>> Map<String, 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
Posted by Mark Phippard <ma...@gmail.com>.
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 Map<String, String>. Should they be adjusted to
> Map<String, 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/