You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2006/12/19 23:26:17 UTC
JavaHL setConfigDirectory() and auth cache problems
On Tue, 7 Nov 2006, Lieven Govaerts wrote:
> I've upgraded some Java scripts to support Subversion 1.4.2 (coming
> from 1.2.3). While executing the script, I find in the current
> working directory a new empty folder structure:
>
> auth\
> svn.simple\
> svn.ssl.usr\
> svn.username\
I can reproduce these symptoms with the following code:
public class test
{
public static void main(String[] args)
{
SVNClientInterface client = new SVNClient();
}
}
...compiled and run like so:
javac -cp $HOME:/usr/local/subversion/lib/svn-javahl/svn-javahl.jar test.java
java -Djava.library.path=/usr/local/subversion/lib -cp $HOME:/usr/local/subversion/lib/svn-javahl/svn-javahl.jar test
> Stepping through the Java code, I found that these folders are
> created from the constructor of
> org.tigris.subversion.javahl.SVNClient:
...
> public SVNClient()
> {
...
> // Ensure that Subversion's config file area and templates exist.
> try
> {
> // Passing an empty string instead of null (which would be
> // more appropriate) prevents earlier versions of the
> // native library (mismatched from the Java bytecode
> // version) from crashing.
> setConfigDirectory("");
> }
> catch (ClientException suppressed)
> {
> // Not an exception-worthy problem, continue on.
> }
> }
...
> I see two problems with the code:
> 1. These folders are supposed to be created in the users home
> directory, not in the current working folder.
Yup.
> 2. Doing some non-trivial initialization in a constructor is against
> best practice (in Java).
Performing this initialization in the constructor is an improvement
over performing the initialization in JavaHL's native library
(e.g. libsvnjavahl-1.so), as it allows SVNClient sub-classes to
determine where the config directory should be (rather than always
attemting to create it in the user's home directory, which is not
always possible). Having the native code always create the config
directory in the same place is rather at odds with the purpose of a
setConfigDirectory() API, and causes serious problems in some
deployment scenarios.
I'm totally open to other options, so long as they don't regress this
functionality.
> Looking at the JavaHL code r20726 (which adds the setConfigDirectory
> call) is most likely the reason of this behavior. If anyone can
> easily see the cause of the problem and solve it that would be nice,
> otherwise I'll have a go later this week.
r20726 was a merge of r20652 from trunk into the 1.4.x branch. r20652
moved a call to svn_config_ensure() from JavaHL's native library --
where it always ran, a behavior which could not be overridden -- into
the SVNClient class's constructor. And so it was Good.
r21316 altered the way SVNClient's constructor made that call to
attempt to allow newer versions of JavaHL's Java libraries to
inter-operate with older versions of Subversion's own core libraries
(a dependency probably created by r20648). In doing so, it introduced
this (mis-)behavior.
Reverting r21316 -- and first r21429, a follow-up -- fixes this
problem, but removes the possibility of inter-op between 1.4.x Java
code and <= 1.3.x Subversion libraries. Unless someone has a better
suggestion, I think we should go ahead make this change on trunk, and
backport it to the 1.4.x line, as it's definitely the lesser of two
evils.
On Fri, 27 Oct 2006, Mark Phippard wrote:
> These could be expected behavior, or they could be JavaHL bugs.
> This is tested on Windows with 1.4.0
>
> 1) JavaHL has a method named setConfigDirectory(String name) that
> allows you to override the location of the directory containing the
> runtime configuration information. From my testing on Windows, it
> appears that this feature works for settings in the config file,
> such as auto props. However, things like the SSL and password cache
> appear to still store in the default location. I would have
> expected all of the configuration area info to be pointed to the new
> location, but it is entirely possible that the feature is only
> intended to let you override the config file.
>
> Does anyone know if this should also work for the cache?
Mike Pilato replied:
> setConfigDirectory() was, I thought, supposed to be the equivalent
> of running --config-dir on the commandline.
>
> So I would expect that, as the auth cache is but one component of
> the .subversion runtime configuration directory, the cache employed
> would follow the recommendation of --config-dir or
> setConfigDirectory().
Mark, I agree with Mike -- SVNClientInterface.setConfigDirectory()
should be dictating the home for the auth cache directory as well as
the config files.
Mark also wrote:
> 2) If the Subversion config area does not exist, then it is created
> automatically when the JavaHL library is loaded. However, if the
> folder does exist, or has incomplete contents, then no attempt is
> made to fill the folder with the correct contents. The way this
> manifests is that when the user is prompted to accept SSL
> credentials, the Accept Permanent does not work because the
> auth/svn.ssl.server folder does not exist. I did not test if
> password cache has same problem, but I'd expect that it would.
>
> I tested scenario #2 with the command line, and it created the
> necessary folders to store the cache info. JavaHL does not.
Mark, is this caused by the bug injected by r21316? Or is this a
different problem?
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by Mark Phippard <ma...@gmail.com>.
On 12/22/06, Daniel Rall <dl...@collab.net> wrote:
>
> Mark wrote:
> > > Dan wrote:
> > > Thanks everyone for the great feedback. How's this look?
> ...
> >
> > +1. Looks like you covered everything.
> >
> > Thanks
>
> I've committed this fix to trunk, and nominated it for backport to the
> 1.4.x branch. Please vote, folks.
I tested with the 1.3 DLL's just to be sure. All worked OK, so I voted for
it.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by Daniel Rall <dl...@collab.net>.
Mark wrote:
> > Dan wrote:
> > Thanks everyone for the great feedback. How's this look?
...
>
> +1. Looks like you covered everything.
>
> Thanks
I've committed this fix to trunk, and nominated it for backport to the
1.4.x branch. Please vote, folks.
- Dan
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by Mark Phippard <ma...@gmail.com>.
On 12/20/06, Daniel Rall <dl...@collab.net> wrote:
>
> On Wed, 20 Dec 2006, C. Michael Pilato wrote:
>
> > Mark Phippard wrote:
> > > Also, I do not believe Windows has a HOME variable, so this will
> > > probably return a null. There is a HOMEPATH variable. By default,
> > > Subversion stores its configuration in %APPDATA%\Subversion. Maybe
> the
> > > function could be enhanced to look for one of these other variables if
> > > HOME does not yield a result.
> >
> > FYI: Our C function svn_user_get_home() checks $HOME on all platforms
> > before falling back to apr_uid_homepath_get() (which does OS-specific
> > stuff), so there is precedent for doing so elsewhere.
>
> Thanks everyone for the great feedback. How's this look?
+1. Looks like you covered everything.
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by Daniel Rall <dl...@collab.net>.
On Wed, 20 Dec 2006, C. Michael Pilato wrote:
> Mark Phippard wrote:
> > Also, I do not believe Windows has a HOME variable, so this will
> > probably return a null. There is a HOMEPATH variable. By default,
> > Subversion stores its configuration in %APPDATA%\Subversion. Maybe the
> > function could be enhanced to look for one of these other variables if
> > HOME does not yield a result.
>
> FYI: Our C function svn_user_get_home() checks $HOME on all platforms
> before falling back to apr_uid_homepath_get() (which does OS-specific
> stuff), so there is precedent for doing so elsewhere.
Thanks everyone for the great feedback. How's this look?
[[[
Try hard to provide an appropriate initial config directory.
While 1.4+ native libraries will Do The Right Thing, mixing 1.3-
native libraries with newer Java bytecode will produce a segfault if a
config directory is not provided.
[ in subversion/bindings/java/javahl/ ]
* src/org/tigris/subversion/javahl/SVNClient.java
Import java.io.File.
(SVNClient): Set the config directory to
determineInitialConfigDir(), allowing it to be overridden by
sub-classes.
(determineInitialConfigDir): Attempt to determine an initial
configuration directory, "%APPDATA%\Subversion" on Windows and
"~/.subversion" on other operating systems.
(getUserHomeDirectory): Return the absolute path to the current
user's home directory,
(getEnv): Attempt to read an environment variable, something which
is not possible with most JREs (1.5 re-introduced this
functionality).
(isOSWindows): Return whether we're running on Windows.
]]]
Index: subversion/bindings/java/javahl/src/org/tigris/subversion/javahl/SVNClient.java
===================================================================
--- subversion/bindings/java/javahl/src/org/tigris/subversion/javahl/SVNClient.java (revision 22774)
+++ subversion/bindings/java/javahl/src/org/tigris/subversion/javahl/SVNClient.java (working copy)
@@ -18,6 +18,7 @@
* @endcopyright
*/
+import java.io.File;
import java.io.OutputStream;
/**
@@ -45,18 +46,99 @@
// Ensure that Subversion's config file area and templates exist.
try
{
- // Passing an empty string instead of null (which would be
- // more appropriate) prevents earlier versions of the
- // native library (mismatched from the Java bytecode
- // version) from crashing.
- setConfigDirectory("");
+ setConfigDirectory(determineInitialConfigDir());
}
catch (ClientException suppressed)
{
// Not an exception-worthy problem, continue on.
}
}
+
/**
+ * Attempt to determine an initial configuration directory,
+ * <code>%APPDATA%\Subversion</code> on Windows and
+ * <code>~/.subversion</code> on other operating systems.
+ *
+ * @return The initial configuration directory, or
+ * <code>null</code> to use the library default. Note that native
+ * library versions older than 1.4 may segfault if we return
+ * <code>null</code>.
+ */
+ protected String determineInitialConfigDir()
+ {
+ String path;
+ if (isOSWindows())
+ {
+ // On Windows, use the %APPDATA%\Subversion directory.
+ path = getEnv("APPDATA");
+ if (path == null)
+ {
+ path = getUserHomeDirectory();
+ if (path != null)
+ {
+ path = new File(path, "Application Data").getPath();
+ }
+ }
+
+ if (path != null)
+ {
+ path = new File(path, "Subversion").getAbsolutePath();
+ }
+ }
+ else
+ {
+ // Everywhere else, use the ~/.subversion directory.
+ path = getUserHomeDirectory();
+ if (path != null)
+ {
+ path = new File(path, ".subversion").getAbsolutePath();
+ }
+ }
+ return path;
+ }
+
+ /**
+ * @return The absolute path to the current user's home directory,
+ * or <code>null</code> if it cannot be determined.
+ */
+ private static String getUserHomeDirectory()
+ {
+ // ### LATER: Wrap the svn_user_get_homedir() API and try it
+ // ### first.
+ String path = System.getProperty("user.home");
+ return (path != null ? path : getEnv("HOME"));
+ }
+
+ /**
+ * @param envVar The name of the environment variable to retrieve.
+ * @return The named environment variable, or <code>null</code> if
+ * it cannot be retrieved.
+ */
+ private static final String getEnv(String envVar)
+ {
+ try
+ {
+ return System.getenv(envVar);
+ }
+ catch (Throwable jreComplaint)
+ {
+ // As an example, Sun JRE 1.4.2_12-b03 throws
+ // java.lang.Error, with the message "getenv no longer
+ // supported, use properties and -D instead: HOME".
+ return null;
+ }
+ }
+
+ /**
+ * @return Whether the current operating system is Windows.
+ */
+ private static final boolean isOSWindows()
+ {
+ String opSys = System.getProperty("os.name");
+ return (opSys.toLowerCase().indexOf("windows") >= 0);
+ }
+
+ /**
* Build the native peer
* @return the adress of the peer
*/
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by "C. Michael Pilato" <cm...@collab.net>.
Mark Phippard wrote:
> Also, I do not believe Windows has a HOME variable, so this will
> probably return a null. There is a HOMEPATH variable. By default,
> Subversion stores its configuration in %APPDATA%\Subversion. Maybe the
> function could be enhanced to look for one of these other variables if
> HOME does not yield a result.
FYI: Our C function svn_user_get_home() checks $HOME on all platforms
before falling back to apr_uid_homepath_get() (which does OS-specific
stuff), so there is precedent for doing so elsewhere.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by Mark Phippard <ma...@gmail.com>.
On 12/20/06, David James <ja...@cs.toronto.edu> wrote:
>
> On 12/20/06, Daniel Rall <dl...@collab.net> wrote:
> > On Wed, 20 Dec 2006, Daniel Rall wrote:
> > ...
> > > Yup, "Java(TM) 2 Runtime Environment, Standard Edition (build
> > > 1.5.0_07-b03)" now works as expected. Thanks for the update, Patrick.
> > > If I can't weasel some native library version checks into JavaHL's
> > > Java code, we'll go with this as a best-effort attempt to maintain
> > > some compatibility with older native library versions.
> >
> [...snip...]
> > + return System.getenv("HOME");
>
> Doesn't the ".subversion" subdirectory need to be tacked on to the end
> here?
Also, I do not believe Windows has a HOME variable, so this will probably
return a null. There is a HOMEPATH variable. By default, Subversion stores
its configuration in %APPDATA%\Subversion. Maybe the function could be
enhanced to look for one of these other variables if HOME does not yield a
result.
Mark
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by David James <ja...@cs.toronto.edu>.
On 12/20/06, Daniel Rall <dl...@collab.net> wrote:
> On Wed, 20 Dec 2006, Daniel Rall wrote:
> ...
> > Yup, "Java(TM) 2 Runtime Environment, Standard Edition (build
> > 1.5.0_07-b03)" now works as expected. Thanks for the update, Patrick.
> > If I can't weasel some native library version checks into JavaHL's
> > Java code, we'll go with this as a best-effort attempt to maintain
> > some compatibility with older native library versions.
>
[...snip...]
> + return System.getenv("HOME");
Doesn't the ".subversion" subdirectory need to be tacked on to the end here?
Cheers,
David
> + }
> + catch (Throwable jreComplaint)
> + {
> + // As an example, Sun JRE 1.4.2_12-b03 throws
> + // java.lang.Error, with the message "getenv no longer
> + // supported, use properties and -D instead: HOME".
> + return null;
> + }
> + }
> +
> + /**
> * Build the native peer
> * @return the adress of the peer
> */
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by Daniel Rall <dl...@collab.net>.
On Wed, 20 Dec 2006, Daniel Rall wrote:
...
> Yup, "Java(TM) 2 Runtime Environment, Standard Edition (build
> 1.5.0_07-b03)" now works as expected. Thanks for the update, Patrick.
> If I can't weasel some native library version checks into JavaHL's
> Java code, we'll go with this as a best-effort attempt to maintain
> some compatibility with older native library versions.
Index: subversion/bindings/java/javahl/src/org/tigris/subversion/javahl/SVNClient.java
===================================================================
--- subversion/bindings/java/javahl/src/org/tigris/subversion/javahl/SVNClient.java (revision 22772)
+++ subversion/bindings/java/javahl/src/org/tigris/subversion/javahl/SVNClient.java (working copy)
@@ -45,18 +45,39 @@
// Ensure that Subversion's config file area and templates exist.
try
{
- // Passing an empty string instead of null (which would be
- // more appropriate) prevents earlier versions of the
- // native library (mismatched from the Java bytecode
- // version) from crashing.
- setConfigDirectory("");
+ setConfigDirectory(determineInitialConfigDir());
}
catch (ClientException suppressed)
{
// Not an exception-worthy problem, continue on.
}
}
+
/**
+ * Attempt to return the user's home directory (not possible with
+ * older JREs).
+ *
+ * @return The initial configuration directory, or
+ * <code>null</code> to use the library default. Note that native
+ * library versions older than 1.4 may segfault if we return
+ * <code>null</code>.
+ */
+ protected String determineInitialConfigDir()
+ {
+ try
+ {
+ return System.getenv("HOME");
+ }
+ catch (Throwable jreComplaint)
+ {
+ // As an example, Sun JRE 1.4.2_12-b03 throws
+ // java.lang.Error, with the message "getenv no longer
+ // supported, use properties and -D instead: HOME".
+ return null;
+ }
+ }
+
+ /**
* Build the native peer
* @return the adress of the peer
*/
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by Daniel Rall <dl...@collab.net>.
On Wed, 20 Dec 2006, Patrick Mayweg wrote:
> Hi Daniel,
>
> Daniel Rall wrote:
> >On Tue, 19 Dec 2006, David James wrote:
...
> >>If we need to be compatible with Subversion 1.3.x DLLs, why don't we
> >>simply fix JavaHL to set the config directory to "$HOME/.subversion"?
> >>There's nothing wrong with being explicit, and this is the only
> >>reliable way to initialize the config dir which is compatible with
> >>both Subversion 1.3.x and 1.4.x DLLs.
> >>
> >
> >Java does not support environment variables, so there is no way to
> >access $HOME.
>
> That is not totally true. System.getEnv(String name) does allows you to
> access the environment variables. It does not work for every version,
> because access to environment variables was deprecated by Sun, but the
> were convinced by the users, that this functionality is needed.
This is the behavior that I'm accustomed to:
$ java test
Exception in thread "main" java.lang.Error: getenv no longer supported, use properties and -D instead: HOME
at java.lang.System.getenv(System.java:691)
at test.main(test.java:8)
I ran this test using "Java(TM) 2 Runtime Environment, Standard
Edition (build 1.4.2_12-b03)", and recall similar disfunctional
behavior with older JREs.
> In Jdk 1.5 they added a new method, which returns all variables in a
> map. The Subversion core libraries handle this default properly on
> trunk, but IIRC, not gracefully enough in the 1.4.x and earlier
> series.
Yup, "Java(TM) 2 Runtime Environment, Standard Edition (build
1.5.0_07-b03)" now works as expected. Thanks for the update, Patrick.
If I can't weasel some native library version checks into JavaHL's
Java code, we'll go with this as a best-effort attempt to maintain
some compatibility with older native library versions.
Since it's only me that'll be embarrassed here, I'm going to CC the
dev list on this conversation. Hope ya don't mind.
- Dan
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by Daniel Rall <dl...@collab.net>.
On Tue, 19 Dec 2006, David James wrote:
> On 12/19/06, Mark Phippard <ma...@gmail.com> wrote:
> >On 12/19/06, Daniel Rall <dl...@collab.net> wrote:
> >
> >> > 2) If the Subversion config area does not exist, then it is created
> >> > automatically when the JavaHL library is loaded. However, if the
> >> > folder does exist, or has incomplete contents, then no attempt is
> >> > made to fill the folder with the correct contents. The way this
> >> > manifests is that when the user is prompted to accept SSL
> >> > credentials, the Accept Permanent does not work because the
> >> > auth/svn.ssl.server folder does not exist. I did not test if
> >> > password cache has same problem, but I'd expect that it would.
> >> >
> >> > I tested scenario #2 with the command line, and it created the
> >> > necessary folders to store the cache info. JavaHL does not.
> >>
> >> Mark, is this caused by the bug injected by r21316? Or is this a
> >> different problem?
> >
> >Hopefully it is the same problem. I have no way of knowing.
> >
> >Wouldn't it be possible to have the code treat "" the same as NULL? I think
> >when we committed it, we thought that it did. Probably because line 2872
> >in
> >SVNClient.cpp:
> >
> > m_configDir = (configDir == NULL ? "" : configDir);
> >
> >I now see that the issue is that this code is AFTER the svn library call
> >has
> >been made. I guess we can live with this if the change is reverted, but I
> >am not sure a JVM crash is the lesser of two evils.
>
> If we need to be compatible with Subversion 1.3.x DLLs, why don't we
> simply fix JavaHL to set the config directory to "$HOME/.subversion"?
> There's nothing wrong with being explicit, and this is the only
> reliable way to initialize the config dir which is compatible with
> both Subversion 1.3.x and 1.4.x DLLs.
Java does not support environment variables, so there is no way to
access $HOME.
The Subversion core libraries handle this default properly on trunk,
but IIRC, not gracefully enough in the 1.4.x and earlier series.
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by David James <ja...@cs.toronto.edu>.
On 12/19/06, Mark Phippard <ma...@gmail.com> wrote:
> On 12/19/06, Daniel Rall <dl...@collab.net> wrote:
>
> > > 2) If the Subversion config area does not exist, then it is created
> > > automatically when the JavaHL library is loaded. However, if the
> > > folder does exist, or has incomplete contents, then no attempt is
> > > made to fill the folder with the correct contents. The way this
> > > manifests is that when the user is prompted to accept SSL
> > > credentials, the Accept Permanent does not work because the
> > > auth/svn.ssl.server folder does not exist. I did not test if
> > > password cache has same problem, but I'd expect that it would.
> > >
> > > I tested scenario #2 with the command line, and it created the
> > > necessary folders to store the cache info. JavaHL does not.
> >
> > Mark, is this caused by the bug injected by r21316? Or is this a
> > different problem?
>
> Hopefully it is the same problem. I have no way of knowing.
>
> Wouldn't it be possible to have the code treat "" the same as NULL? I think
> when we committed it, we thought that it did. Probably because line 2872 in
> SVNClient.cpp:
>
> m_configDir = (configDir == NULL ? "" : configDir);
>
> I now see that the issue is that this code is AFTER the svn library call has
> been made. I guess we can live with this if the change is reverted, but I
> am not sure a JVM crash is the lesser of two evils.
If we need to be compatible with Subversion 1.3.x DLLs, why don't we
simply fix JavaHL to set the config directory to "$HOME/.subversion"?
There's nothing wrong with being explicit, and this is the only
reliable way to initialize the config dir which is compatible with
both Subversion 1.3.x and 1.4.x DLLs.
Cheers,
David
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by "C. Michael Pilato" <cm...@collab.net>.
Mark Phippard wrote:
> Wouldn't it be possible to have the code treat "" the same as NULL? I
> think when we committed it, we thought that it did. Probably because
> line 2872 in SVNClient.cpp:
>
> m_configDir = (configDir == NULL ? "" : configDir);
I see "" and NULL meaning two entirely different things, "" meaning
"read the configuration from the current working directory" and NULL
meaning "I want a the default configuration, so don't even try to read
the disk".
That said, I have *no* idea what standards the JavaHL code has for
itself regarding path handling of that sort.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: JavaHL setConfigDirectory() and auth cache problems
Posted by Mark Phippard <ma...@gmail.com>.
On 12/19/06, Daniel Rall <dl...@collab.net> wrote:
> > 2) If the Subversion config area does not exist, then it is created
> > automatically when the JavaHL library is loaded. However, if the
> > folder does exist, or has incomplete contents, then no attempt is
> > made to fill the folder with the correct contents. The way this
> > manifests is that when the user is prompted to accept SSL
> > credentials, the Accept Permanent does not work because the
> > auth/svn.ssl.server folder does not exist. I did not test if
> > password cache has same problem, but I'd expect that it would.
> >
> > I tested scenario #2 with the command line, and it created the
> > necessary folders to store the cache info. JavaHL does not.
>
> Mark, is this caused by the bug injected by r21316? Or is this a
> different problem?
Hopefully it is the same problem. I have no way of knowing.
Wouldn't it be possible to have the code treat "" the same as NULL? I think
when we committed it, we thought that it did. Probably because line 2872 in
SVNClient.cpp:
m_configDir = (configDir == NULL ? "" : configDir);
I now see that the issue is that this code is AFTER the svn library call has
been made. I guess we can live with this if the change is reverted, but I
am not sure a JVM crash is the lesser of two evils.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/