Re: svn commit: r24791 - in trunk/subversion/bindings/javahl: native src/org/tigris/subversion/javahl src/org/tigris/subversion/javahl/tests

On Thu, 26 Apr 2007, Daniel Rall wrote:

> On Thu, 26 Apr 2007, Hyrum K. Wright wrote:
>
> > Daniel Rall wrote:
> > > On Thu, 26 Apr 2007, hwright@tigris.org wrote:
> > > ...
> > >> JavaHL: Eliminate duplicate native implementations of propertyCreate(), and
> > >> implement one version as a wrapper around the other. Also, modify the
> > >> properties test to use propertyCreate() as well as propertySet().
> > > ...
> > >> --- trunk/subversion/bindings/javahl/native/org_tigris_subversion_javahl_SVNClient.cpp (original)
> > >> +++ trunk/subversion/bindings/javahl/native/org_tigris_subversion_javahl_SVNClient.cpp Thu Apr 26 09:00:39 2007
> ...
> > >> - JNIByteArray value(jvalue);
> > >> - if (JNIUtil::isExceptionThrown())
> > >> - return;
> > >> -
> > >> - cl->propertyCreate(path, name, (const char *)value.getBytes(),
> > >> - jrecurse ? true:false,
> > >> - jforce ? true:false);
> > >> -}
> ...
> > >> --- trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/SVNClient.java (original)
> > >> +++ trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/SVNClient.java Thu Apr 26 09:00:39 2007
> > >> @@ -1447,9 +1447,12 @@
> > >> * @throws ClientException
> > >> * @since 1.2
> > >> */
> > >> - public native void propertyCreate(String path, String name, byte[] value,
> > >> - boolean recurse, boolean force)
> > >> - throws ClientException;
> > >> + public void propertyCreate(String path, String name, byte[] value,
> > >> + boolean recurse, boolean force)
> > >> + throws ClientException
> > >> + {
> > >> + propertyCreate(path, name, new String(value), recurse, force);
> > >> + }
> > > ...
> > >
> > > Isn't "new String(value)" doing an implicit byte -> char conversion in
> > > the JVM's default character set? Could this be lossy?
> > >
> > > It might be safer to make this change the other way around...
> >
> > Sure. The motivation for doing it this way was consistency with a
> > similar construct in propertySet(). I'd be happy to change them both if
> > needed.
>
> While I'm not absolutely sure what the Right Thing is here, getting
> the byte[] representation of a Java String using the JVM's default
> encoding (which unfortunately might not be UTF-8), then basically
> transforming each byte in that array into a character, is definitely
> not correct.

To clarify: That's what we used to do.

Now we seem to do the opposite. However, if we don't know what
encoding to create a String from using those bytes, using the JVM's
default encoding may not be correct, either.