Harmony
  1. Harmony
  2. HARMONY-1751

[classlib][prefs] Classlib test org.apache.harmony.prefs.tests.java.util.prefs.PreferencesTest.testSystemNodeForPackage() fails

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Classlib
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      The test fails for me on Windows on both j9 and drlvm with the following assertion:

      ant -Dbuild.module=prefs -Dtest.case=PreferencesTest test

      testSystemNodeForPackage Error N/A
      (J9)
      java.lang.SecurityException at java.util.prefs.RegistryPreferencesImpl.childSpi(RegistryPreferencesImpl.java:116) at java.util.prefs.AbstractPreferences.getNodeFromBackend(AbstractPreferences.java:645) at java.util.prefs.AbstractPreferences.nodeImpl(AbstractPreferences.java:626) at java.util.prefs.AbstractPreferences.node(AbstractPreferences.java:597) at java.util.prefs.Preferences.systemNodeForPackage(Preferences.java:767) at org.apache.harmony.prefs.tests.java.util.prefs.PreferencesTest.testSystemNodeForPackage(PreferencesTest.java:62) at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:25)

      (drlvm)
      java.lang.SecurityException at java.util.prefs.RegistryPreferencesImpl.childSpi(RegistryPreferencesImpl.java:116) at java.util.prefs.AbstractPreferences.getNodeFromBackend(AbstractPreferences.java:644) at java.util.prefs.AbstractPreferences.nodeImpl(AbstractPreferences.java:625) at java.util.prefs.AbstractPreferences.node(AbstractPreferences.java:595) at java.util.prefs.Preferences.systemNodeForPackage(Preferences.java:767) at org.apache.harmony.prefs.tests.java.util.prefs.PreferencesTest.testSystemNodeForPackage(PreferencesTest.java:62) at java.lang.reflect.VMReflection.invokeMethod(Native Method)

      1. fixFor_getNode.patch
        0.7 kB
        Stepan Mishura
      2. Harmony-1751.patch
        3 kB
        Ilya Okomin
      3. H-1751_PreferencesTest_updated.patch
        1 kB
        Elena Semukhina
      4. PreferencesTest.patch
        2 kB
        Elena Semukhina
      5. TestPref.java
        0.2 kB
        Elena Semukhina

        Activity

        Hide
        Alexei Fedotov added a comment -

        [drlvm][unit] Blocks http://wiki.apache.org/harmony/Unit_Tests_Pass_on_DRLVM

        Stepan, could you please help Elena to resolve this issue?

        Show
        Alexei Fedotov added a comment - [drlvm] [unit] Blocks http://wiki.apache.org/harmony/Unit_Tests_Pass_on_DRLVM Stepan, could you please help Elena to resolve this issue?
        Hide
        Elena Semukhina added a comment -

        I extracted a simple test from PreferencesTest and ran it on j9. I got SecurityException on Preferences.systemNodeForPackage(Object.class). The spec says that this method throws SecurityException if a security manager is present and it denies RuntimePermission("preferences"). The test shows that SecurityManager is null but Exception is thrown. I don't have .java.policy file in my home directory. The test has been run on Windows XP:

        C:\users\esemukhi\Test\Prefs>C:\users\esemukhi\svn1\classlib\trunk\deploy\jdk\jr
        e\bin\java TestPref
        SecurityManager = null
        Exception in thread "main" java.lang.SecurityException
        at java.util.prefs.RegistryPreferencesImpl.childSpi(RegistryPreferencesI
        mpl.java:116)
        at java.util.prefs.AbstractPreferences.getNodeFromBackend(AbstractPrefer
        ences.java:645)
        at java.util.prefs.AbstractPreferences.nodeImpl(AbstractPreferences.java
        :626)
        at java.util.prefs.AbstractPreferences.node(AbstractPreferences.java:597
        )
        at java.util.prefs.Preferences.systemNodeForPackage(Preferences.java:767
        )
        at TestPref.main(TestPref.java:7)

        Show
        Elena Semukhina added a comment - I extracted a simple test from PreferencesTest and ran it on j9. I got SecurityException on Preferences.systemNodeForPackage(Object.class). The spec says that this method throws SecurityException if a security manager is present and it denies RuntimePermission("preferences"). The test shows that SecurityManager is null but Exception is thrown. I don't have .java.policy file in my home directory. The test has been run on Windows XP: C:\users\esemukhi\Test\Prefs>C:\users\esemukhi\svn1\classlib\trunk\deploy\jdk\jr e\bin\java TestPref SecurityManager = null Exception in thread "main" java.lang.SecurityException at java.util.prefs.RegistryPreferencesImpl.childSpi(RegistryPreferencesI mpl.java:116) at java.util.prefs.AbstractPreferences.getNodeFromBackend(AbstractPrefer ences.java:645) at java.util.prefs.AbstractPreferences.nodeImpl(AbstractPreferences.java :626) at java.util.prefs.AbstractPreferences.node(AbstractPreferences.java:597 ) at java.util.prefs.Preferences.systemNodeForPackage(Preferences.java:767 ) at TestPref.main(TestPref.java:7)
        Hide
        Elena Semukhina added a comment -

        Test to reproduce the issue attached.

        Show
        Elena Semukhina added a comment - Test to reproduce the issue attached.
        Hide
        Boris Kuznetsov added a comment -

        The SecurityException is thrown in RegistryPreferencesImpl.childSpi() after successful security check in Preferences.systemNodeForPackage():
        result.newNode = getNode(path, encodeWindowsStr(name).getBytes(), result.userNode, error);
        if (error[ERROR_CODE] == RETURN_ACCESS_DENIED)

        { throw new SecurityException(); }

        Error code RETURN_ACCESS_DENIED is returned from native code (PreferencesImpl.c). When openRegKey() calls the RegOpenKeyEx function to open the registry key, the system checks the requested access rights. If user does not have the correct access to the registry key, the open operation fails.
        It looks like Win configuration problem.

        Show
        Boris Kuznetsov added a comment - The SecurityException is thrown in RegistryPreferencesImpl.childSpi() after successful security check in Preferences.systemNodeForPackage(): result.newNode = getNode(path, encodeWindowsStr(name).getBytes(), result.userNode, error); if (error [ERROR_CODE] == RETURN_ACCESS_DENIED) { throw new SecurityException(); } Error code RETURN_ACCESS_DENIED is returned from native code (PreferencesImpl.c). When openRegKey() calls the RegOpenKeyEx function to open the registry key, the system checks the requested access rights. If user does not have the correct access to the registry key, the open operation fails. It looks like Win configuration problem.
        Hide
        Alexei Fedotov added a comment -

        My suggestion is to make the test passed in case of RETURN_ACCESS_DENIED and print a warning message. Boris, does it make sence?

        Show
        Alexei Fedotov added a comment - My suggestion is to make the test passed in case of RETURN_ACCESS_DENIED and print a warning message. Boris, does it make sence?
        Hide
        Elena Semukhina added a comment -

        I made some experiments on RI.
        On RI TestPref prints only a warning and does not throw SecurityException:

        Oct 10, 2006 2:21:15 PM java.util.prefs.WindowsPreferences <init>
        WARNING: Could not create windows registry node Software\JavaSoft\Prefs\java at
        root 0x80000002. Windows RegCreateKeyEx(...) returned error code 5.
        Oct 10, 2006 2:21:15 PM java.util.prefs.WindowsPreferences WindowsRegOpenKey1
        WARNING: Trying to recreate Windows registry node Software\JavaSoft\Prefs\java a
        t root 0x80000002.
        Oct 10, 2006 2:21:15 PM java.util.prefs.WindowsPreferences openKey
        WARNING: Could not open windows registry node Software\JavaSoft\Prefs\java at ro
        ot 0x80000002. Windows RegOpenKey(...) returned error code 2.

        On the other hand, PreferencesTest fails on RI while invoking childrenNames() method with BackingStoreException while it is not thrown on Harmony classlib:

        1) testSystemNodeForPackage(org.apache.harmony.prefs.tests.java.util.prefs.Prefe
        rencesTest)java.util.prefs.BackingStoreException: Could not open windowsregistry
        node Software\JavaSoft\Prefs\java\lang at root 0x80000002.
        at java.util.prefs.WindowsPreferences.childrenNamesSpi(WindowsPreference
        s.java:719)
        at java.util.prefs.AbstractPreferences.childrenNames(AbstractPreferences
        .java:699)
        at org.apache.harmony.prefs.tests.java.util.prefs.PreferencesTest.testSy
        stemNodeForPackage(PreferencesTest.java:73)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.
        java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAcces
        sorImpl.java:25)

        Possibly we should change j.u.prefs.Preferences behavior to be compatible with RI.

        Show
        Elena Semukhina added a comment - I made some experiments on RI. On RI TestPref prints only a warning and does not throw SecurityException: Oct 10, 2006 2:21:15 PM java.util.prefs.WindowsPreferences <init> WARNING: Could not create windows registry node Software\JavaSoft\Prefs\java at root 0x80000002. Windows RegCreateKeyEx(...) returned error code 5. Oct 10, 2006 2:21:15 PM java.util.prefs.WindowsPreferences WindowsRegOpenKey1 WARNING: Trying to recreate Windows registry node Software\JavaSoft\Prefs\java a t root 0x80000002. Oct 10, 2006 2:21:15 PM java.util.prefs.WindowsPreferences openKey WARNING: Could not open windows registry node Software\JavaSoft\Prefs\java at ro ot 0x80000002. Windows RegOpenKey(...) returned error code 2. On the other hand, PreferencesTest fails on RI while invoking childrenNames() method with BackingStoreException while it is not thrown on Harmony classlib: 1) testSystemNodeForPackage(org.apache.harmony.prefs.tests.java.util.prefs.Prefe rencesTest)java.util.prefs.BackingStoreException: Could not open windowsregistry node Software\JavaSoft\Prefs\java\lang at root 0x80000002. at java.util.prefs.WindowsPreferences.childrenNamesSpi(WindowsPreference s.java:719) at java.util.prefs.AbstractPreferences.childrenNames(AbstractPreferences .java:699) at org.apache.harmony.prefs.tests.java.util.prefs.PreferencesTest.testSy stemNodeForPackage(PreferencesTest.java:73) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl. java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAcces sorImpl.java:25) Possibly we should change j.u.prefs.Preferences behavior to be compatible with RI.
        Hide
        Alexei Fedotov added a comment -

        I believe both the test and the implementation should be fixed to be compatible with RI.

        Show
        Alexei Fedotov added a comment - I believe both the test and the implementation should be fixed to be compatible with RI.
        Hide
        Elena Semukhina added a comment -

        I attached the patch for PreferencesTest which ensures that testSystemNodeForPackage() passes on RI.

        Show
        Elena Semukhina added a comment - I attached the patch for PreferencesTest which ensures that testSystemNodeForPackage() passes on RI.
        Hide
        Elena Semukhina added a comment -

        I reattached the patch for the test (H-1751_PreferencesTest_updated.patch) because the old one is not applicable to PreferenceTest.java r. 463643.

        Show
        Elena Semukhina added a comment - I reattached the patch for the test (H-1751_PreferencesTest_updated.patch) because the old one is not applicable to PreferenceTest.java r. 463643.
        Hide
        Ilya Okomin added a comment -

        This issue can be reproduced on desktop with reduced user's rights.

        I created patch that resolves this compatibility issue, please check it:
        instead of SecurityExceptions as Harmony threw before there will be warning messages printed to the system output.

        Show
        Ilya Okomin added a comment - This issue can be reproduced on desktop with reduced user's rights. I created patch that resolves this compatibility issue, please check it: instead of SecurityExceptions as Harmony threw before there will be warning messages printed to the system output.
        Hide
        Vladimir Ivanov added a comment -

        Currently, my CC on winXP failed due to this issue.
        Could somebody from committers integrate the patch?

        Show
        Vladimir Ivanov added a comment - Currently, my CC on winXP failed due to this issue. Could somebody from committers integrate the patch?
        Hide
        Stepan Mishura added a comment -

        Ilya, could you describe how I can reproduce the failure? How I should change my rigths to see the failure?

        Thanks,
        Stepan.

        Show
        Stepan Mishura added a comment - Ilya, could you describe how I can reproduce the failure? How I should change my rigths to see the failure? Thanks, Stepan.
        Hide
        Ilya Okomin added a comment -

        Stepan,
        Taking into account fact that Harmony returns error code in native PreferencesImpl.c -> Java_java_util_prefs_RegistryPreferencesImpl_getNode() call where registry Key is to be created;
        and RI stack trace "Could not create windows registry node Software\JavaSoft\Prefs\java at
        root 0x80000002. Windows RegCreateKeyEx(...) returned error code 5"
        I consider that it is related to the registry restrictions.

        Thanks,
        Ilya.

        Show
        Ilya Okomin added a comment - Stepan, Taking into account fact that Harmony returns error code in native PreferencesImpl.c -> Java_java_util_prefs_RegistryPreferencesImpl_getNode() call where registry Key is to be created; and RI stack trace "Could not create windows registry node Software\JavaSoft\Prefs\java at root 0x80000002. Windows RegCreateKeyEx(...) returned error code 5" I consider that it is related to the registry restrictions. Thanks, Ilya.
        Hide
        Stepan Mishura added a comment -

        Hi,

        I've managed to reproduce the failure - I've denied full control for me for the following registry entry:
        HKEY_LOCAL_MACHINE / SOFTWARE / JavaSoft / Prefs

        So from my point of view the problem is in your testing env. not in the test. Objections?

        Thanks,
        Stepan.

        Show
        Stepan Mishura added a comment - Hi, I've managed to reproduce the failure - I've denied full control for me for the following registry entry: HKEY_LOCAL_MACHINE / SOFTWARE / JavaSoft / Prefs So from my point of view the problem is in your testing env. not in the test. Objections? Thanks, Stepan.
        Hide
        Ilya Okomin added a comment -

        Thank you, Stepan for your investigation.
        I'm also sure that the cause of this failure is testing environment. But from the other point of view there is a question of compatibility presents here. If we strive for compatibility with RI we should apply suggested patch, otherwise in the mentioned cases Harmony will throw exceptions while RI will just print warning messages. Any thoughts?

        Regards, Ilya.

        Show
        Ilya Okomin added a comment - Thank you, Stepan for your investigation. I'm also sure that the cause of this failure is testing environment. But from the other point of view there is a question of compatibility presents here. If we strive for compatibility with RI we should apply suggested patch, otherwise in the mentioned cases Harmony will throw exceptions while RI will just print warning messages. Any thoughts? Regards, Ilya.
        Hide
        Elena Semukhina added a comment -

        I agree with Ilya here. The test passes on RI and fails on Harmony because it throws SecurityException which is against spec.
        So there is a bug in implementation. The test is incorrect also because it does not catch BackingStoreException which is thrown by both RI and Harmony when the system registry restricts user's permissions.

        Show
        Elena Semukhina added a comment - I agree with Ilya here. The test passes on RI and fails on Harmony because it throws SecurityException which is against spec. So there is a bug in implementation. The test is incorrect also because it does not catch BackingStoreException which is thrown by both RI and Harmony when the system registry restricts user's permissions.
        Hide
        Stepan Mishura added a comment -

        Hi Ilya, Elena.

        Sorry for delay with patches evaluation; I agree that Harmony implementation is not compatible with RI and we should fix it. The problems I see here are that the class behavior is OS-depended, not explicitly specified (we don't know how SPI methods were implemented by RI) and hard to test the class implementation (we depend on backing store's state). I was going to fix patches myself but I've realized that there are a lot of changes and it'd better to discuss them first.

        1) Let's review the patch for RegistryPreferencesImpl.java first - it fixes the next methods:

        • childSpi(String name);
        • putSpi(String name, String value),
        • removeNodeSpi()

        The patch suggests ignoring error code returned by native API and printing a warning instead of throwing SecurityException. I disagree with this approach.

        The spec. for AbstractPreferences class says: "The remaining SPI methods putSpi(String,String), removeSpi(String) and childSpi(String) have more complicated exception behavior. They are not specified to throw BackingStoreException, as they can generally obey their contracts even if the backing store is unavailable ... There is one circumstance under which putSpi, removeSpi and childSpi should throw an exception: if the caller lacks sufficient privileges on the underlying operating system to perform the requested operation. This will, for instance, occur on most systems if a non-privileged user attempts to modify system preferences..."

        So we should throw SecurityException if error code is ERROR_ACCESS_DENIED.

        For example, you may wish to try the following test with RI:

        • open registry with registry editor
        • find HKEY_LOCAL_MACHINE / SOFTWARE / JavaSoft / Prefs entry
        • deny all permission for you
        • run a test that only invokes: Preferences.systemNodeForPackage(Object.class);

        RI will throw SecurityException. And Harmony with the suggested patch applied will print a warning only.

        From my point of view a native code should be fixed first. For example, childSpi(String) methods invokes getNode() that has the following algorithm:

        • invoke openRegKey() that in turn tries to open registry key and if it fails to open the key it creates it.
        • checks error code returned by openRegKey() and if err!=ERROR_SUCCESS returns
        • the next step is confusing for me: so we have err==ERROR_SUCCESS now but the method invokes RegCreateKeyEx again and returns dwDisposition==REG_CREATED_NEW_KEY value (that always is false in this case)

        I'd fix it in the next way: first try to create a registry key and check returned error code. If the err== ERROR_ACCESS_DENIED just simply verify if the key exists (by invoking RegOpenKeyEx). Otherwise return the error code.

        A minor note: RI uses logging API for printing warnings instead of System.out. Frankly, I'm now quite sure what we should use.

        2) Now the patch for the test.

        I agree that behavior depends on backing store's state so BackingStoreException can be thrown. (BTW, I'd also catch SecurityException for systemNodeForPackage(Object.class) see example above)

        But I was thinking how to make the testing more predictable (I like automated testing and dislike to edit registry keys by hands). For example, just crazy idea, to develop a native library for testing that let the test verify registry permissions. So the test can check permissions for registry keys first and them it can verify that a method does the right things. Is this possible?

        A minor note: I'd pass to systemNodeForPackage() PreferencesTest.class instead of Object.class. So if the test fails to remove created key it is clear who put this garbage to the registry.

        Thoughts?

        Thanks,
        Stepan.

        Show
        Stepan Mishura added a comment - Hi Ilya, Elena. Sorry for delay with patches evaluation; I agree that Harmony implementation is not compatible with RI and we should fix it. The problems I see here are that the class behavior is OS-depended, not explicitly specified (we don't know how SPI methods were implemented by RI) and hard to test the class implementation (we depend on backing store's state). I was going to fix patches myself but I've realized that there are a lot of changes and it'd better to discuss them first. 1) Let's review the patch for RegistryPreferencesImpl.java first - it fixes the next methods: childSpi(String name); putSpi(String name, String value), removeNodeSpi() The patch suggests ignoring error code returned by native API and printing a warning instead of throwing SecurityException. I disagree with this approach. The spec. for AbstractPreferences class says: "The remaining SPI methods putSpi(String,String), removeSpi(String) and childSpi(String) have more complicated exception behavior. They are not specified to throw BackingStoreException, as they can generally obey their contracts even if the backing store is unavailable ... There is one circumstance under which putSpi, removeSpi and childSpi should throw an exception: if the caller lacks sufficient privileges on the underlying operating system to perform the requested operation. This will, for instance, occur on most systems if a non-privileged user attempts to modify system preferences..." So we should throw SecurityException if error code is ERROR_ACCESS_DENIED. For example, you may wish to try the following test with RI: open registry with registry editor find HKEY_LOCAL_MACHINE / SOFTWARE / JavaSoft / Prefs entry deny all permission for you run a test that only invokes: Preferences.systemNodeForPackage(Object.class); RI will throw SecurityException. And Harmony with the suggested patch applied will print a warning only. From my point of view a native code should be fixed first. For example, childSpi(String) methods invokes getNode() that has the following algorithm: invoke openRegKey() that in turn tries to open registry key and if it fails to open the key it creates it. checks error code returned by openRegKey() and if err!=ERROR_SUCCESS returns the next step is confusing for me: so we have err==ERROR_SUCCESS now but the method invokes RegCreateKeyEx again and returns dwDisposition==REG_CREATED_NEW_KEY value (that always is false in this case) I'd fix it in the next way: first try to create a registry key and check returned error code. If the err== ERROR_ACCESS_DENIED just simply verify if the key exists (by invoking RegOpenKeyEx). Otherwise return the error code. A minor note: RI uses logging API for printing warnings instead of System.out. Frankly, I'm now quite sure what we should use. 2) Now the patch for the test. I agree that behavior depends on backing store's state so BackingStoreException can be thrown. (BTW, I'd also catch SecurityException for systemNodeForPackage(Object.class) see example above) But I was thinking how to make the testing more predictable (I like automated testing and dislike to edit registry keys by hands). For example, just crazy idea, to develop a native library for testing that let the test verify registry permissions. So the test can check permissions for registry keys first and them it can verify that a method does the right things. Is this possible? A minor note: I'd pass to systemNodeForPackage() PreferencesTest.class instead of Object.class. So if the test fails to remove created key it is clear who put this garbage to the registry. Thoughts? Thanks, Stepan.
        Hide
        Ilya Okomin added a comment -

        Stepan, let me add my 0,02$.
        I've checked the example you'd suggested...behavior of RI and Harmony really differs.

        Concerning your note about fixes in native code...please revise it again. I'm not sure there is something wong there.
        IMO algorithm steps are following:

        • invoke openRegKey("root", "path") where "root" == HKEY_CURRENT_USER or HKEY_LOCAL_MACHINE depending on the arguments. This method in turn tries to open registry key "path" and if it fails to open the key it creates it, if SUCCESS - "hKey" variable for "path" is initiated.
        • checks error code returned by openRegKey("root", "path") and if err!=ERROR_SUCCESS returns,
        • the next step to invoke RegCreateKeyEx again but with another parameters ("hKey", "name") and returns dwDisposition==REG_CREATED_NEW_KEY value that is false in the case, when "name" key couldn't have been created for "hKey" handle.
          Everything looks fine for me.

        If I understand right, if spec. for AbstractPreferences says that "putSpi, removeSpi and childSpi should throw an exception: if the caller lacks sufficient privileges...While implementations are not required to throw an exception under these circumstances, they are encouraged to do so. A SecurityException would be appropriate." we can follow the spec and treat this case as non-bug difference. Stepan, Elena, what do you think about this conclusion?

        Thanks,
        Ilya.

        Show
        Ilya Okomin added a comment - Stepan, let me add my 0,02$. I've checked the example you'd suggested...behavior of RI and Harmony really differs. Concerning your note about fixes in native code...please revise it again. I'm not sure there is something wong there. IMO algorithm steps are following: invoke openRegKey("root", "path") where "root" == HKEY_CURRENT_USER or HKEY_LOCAL_MACHINE depending on the arguments. This method in turn tries to open registry key "path" and if it fails to open the key it creates it, if SUCCESS - "hKey" variable for "path" is initiated. checks error code returned by openRegKey("root", "path") and if err!=ERROR_SUCCESS returns, the next step to invoke RegCreateKeyEx again but with another parameters ("hKey", "name") and returns dwDisposition==REG_CREATED_NEW_KEY value that is false in the case, when "name" key couldn't have been created for "hKey" handle. Everything looks fine for me. If I understand right, if spec. for AbstractPreferences says that "putSpi, removeSpi and childSpi should throw an exception: if the caller lacks sufficient privileges...While implementations are not required to throw an exception under these circumstances, they are encouraged to do so. A SecurityException would be appropriate." we can follow the spec and treat this case as non-bug difference. Stepan, Elena, what do you think about this conclusion? Thanks, Ilya.
        Hide
        Alexei Fedotov added a comment -

        Ilya,

        Consider Elena's test TestPref.java. Elena reported that it behaves differently on RI and Harmony. As far as I understand RI behavior is ok from the point of the specification, isn't it? If yes, does it make sense modify Harmony API to behave in a same way as RI for this test?

        Show
        Alexei Fedotov added a comment - Ilya, Consider Elena's test TestPref.java. Elena reported that it behaves differently on RI and Harmony. As far as I understand RI behavior is ok from the point of the specification, isn't it? If yes, does it make sense modify Harmony API to behave in a same way as RI for this test?
        Hide
        Elena Semukhina added a comment -

        All,

        the bug was filed because Harmony throws SecurityException on Preferences.systemNodeForPackage() while RI does not. On one hand, spec for systemNodeForPackage() does not specify throwing it (if there is no a security manager). On the other hand, the implementation of systemNodeForPackage() relies on RegistryPreferencesImpl.childSpi() and childSpi is recommended to throw SecurityException according to AbstractPreferences spec.

        If we agree to throw SecurityException in childSpi, can we catch it in systemNodeForPackage() and just print a warning to be compatible with RI?

        Show
        Elena Semukhina added a comment - All, the bug was filed because Harmony throws SecurityException on Preferences.systemNodeForPackage() while RI does not. On one hand, spec for systemNodeForPackage() does not specify throwing it (if there is no a security manager). On the other hand, the implementation of systemNodeForPackage() relies on RegistryPreferencesImpl.childSpi() and childSpi is recommended to throw SecurityException according to AbstractPreferences spec. If we agree to throw SecurityException in childSpi, can we catch it in systemNodeForPackage() and just print a warning to be compatible with RI?
        Hide
        Stepan Mishura added a comment -

        Hi

        Ilya, thanks for your comments on native code - you are right that getNode() algorithm is correct. However I still think that the bug in native code and the following line is incorrect:
        err = openRegKey (env, jpath, jUserNode, &hKey, KEY_QUERY_VALUE);

        I assume that Elena didn't have KEY_QUERY_VALUE permission on test machine. Please review and try a patch (fixFor_getNode.patch) to fix the issue with Preferences.systemNodeForPackage(Object.class);

        Steps to verify the fix (should work on Harmony and RI):
        1) Apply the patch and do 'ant build-native'
        2) create test that only invokes: Preferences.systemNodeForPackage(Object.class);
        3) deny all permission for HKEY_LOCAL_MACHINE / SOFTWARE / JavaSoft / Prefs
        4) run the test - it MUST fail with SecurityException
        5) allow 'Create Subkey' permission only for HKEY_LOCAL_MACHINE / SOFTWARE / JavaSoft / Prefs
        6) run the test - it MUST pass
        7) allow all permissions only for HKEY_LOCAL_MACHINE / SOFTWARE / JavaSoft / Prefs
        8) press refresh (F5) on 'Prefs' entry - java/lang sub-entries MUST exist

        Thanks,
        Stepan.

        Show
        Stepan Mishura added a comment - Hi Ilya, thanks for your comments on native code - you are right that getNode() algorithm is correct. However I still think that the bug in native code and the following line is incorrect: err = openRegKey (env, jpath, jUserNode, &hKey, KEY_QUERY_VALUE); I assume that Elena didn't have KEY_QUERY_VALUE permission on test machine. Please review and try a patch (fixFor_getNode.patch) to fix the issue with Preferences.systemNodeForPackage(Object.class); Steps to verify the fix (should work on Harmony and RI): 1) Apply the patch and do 'ant build-native' 2) create test that only invokes: Preferences.systemNodeForPackage(Object.class); 3) deny all permission for HKEY_LOCAL_MACHINE / SOFTWARE / JavaSoft / Prefs 4) run the test - it MUST fail with SecurityException 5) allow 'Create Subkey' permission only for HKEY_LOCAL_MACHINE / SOFTWARE / JavaSoft / Prefs 6) run the test - it MUST pass 7) allow all permissions only for HKEY_LOCAL_MACHINE / SOFTWARE / JavaSoft / Prefs 8) press refresh (F5) on 'Prefs' entry - java/lang sub-entries MUST exist Thanks, Stepan.
        Hide
        Ilya Okomin added a comment -

        Hello Stepan!
        Sorry for delay in my reply.

        I've checked your fix:
        1. Suggested scenario works (however ../JavaSoft / Prefs key refreshed for me only when I had allowed Enumerate Subkeys and Query Value permissions to this key). This scenario worked only for the first time, when java/lang key subtree was created. If you are implementing the scenario when java/lang was already created - SecurityException is thrown on Harmony.
        2. With only allowed 'Create Subkey' permission testSystemNodeForPackage passed for me( only for the first run, next runs failed with SecurityException ) on Harmony, but it failed on RI with exception:

        Could not open windows registry node Software\JavaSoft\Prefs\java\lang at root 0x80000002: Access denied
        java.lang.SecurityException: Could not open windows registry node Software\JavaSoft\Prefs\java\lang at root 0x80000002: Access denied at java.util.prefs.WindowsPreferences.openKey(WindowsPreferences.java:496) at java.util.prefs.WindowsPreferences.openKey(WindowsPreferences.java:463) at java.util.prefs.WindowsPreferences.openKey(WindowsPreferences.java:449) at java.util.prefs.WindowsPreferences.childrenNamesSpi(WindowsPreferences.java:717) at java.util.prefs.AbstractPreferences.childrenNames(AbstractPreferences.java:699) at org.apache.harmony.prefs.tests.java.util.prefs.PreferencesTest.testSystemNodeForPackage(PreferencesTest.java:74) at jrockit.reflect.VirtualNativeMethodInvoker.invoke(Ljava.lang.Object;[Ljava.lang.Object;)Ljava.lang.Object;(Unknown Source)

        3. With combination of permissions, that I had on the machine for testing (I suppose Elena had the same permissions set): Allow permissions set:

        • Query Value
        • Enumerate Subkeys
        • Notify
        • Read Control

        with your fix testSystemNodeForPackage passed for me on RI, but it failed on Harmony with exception:

        java.lang.SecurityException at java.util.prefs.RegistryPreferencesImpl.childSpi(RegistryPreferencesImpl.java:116) at java.util.prefs.AbstractPreferences.getNodeFromBackend(AbstractPreferences.java:646) at java.util.prefs.AbstractPreferences.nodeImpl(AbstractPreferences.java:626) at java.util.prefs.AbstractPreferences.node(AbstractPreferences.java:597) at java.util.prefs.Preferences.systemNodeForPackage(Preferences.java:767) at org.apache.harmony.prefs.tests.java.util.prefs.PreferencesTest.testSystemNodeForPackage(PreferencesTest.java:62) at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:25)

        I want to add, despite test passed on RI there was a set of warnings printed to the log:

        [junit] WARNING: Could not create windows registry node Software\JavaSoft\Prefs\java at root 0x80000002. Windows RegCreateKeyEx(...) returned error code 5.
        [junit] 05.12.2006 16:10:22 java.util.prefs.WindowsPreferences WindowsRegOpenKey1
        [junit] WARNING: Trying to recreate Windows registry node Software\JavaSoft\Prefs\java at root 0x80000002.
        [junit] 05.12.2006 16:10:22 java.util.prefs.WindowsPreferences openKey
        [junit] WARNING: Could not open windows registry node Software\JavaSoft\Prefs\java at root 0x80000002. Windows RegOpenKey(...) returned error code 2.
        [junit] Preferences p = System Preference Node: /java/lang
        ...etc...

        Summary:
        Test passes for users with allowed permissions. Spec keeps silence how to handle cases with certain users permissions and it clearly says that implementations are encouraged to throw SecurityException if the caller lacks sufficient privileges. Fix for this issue doesn't seem so easy to me. I would suggest to leave this as non-bug difference again or we can discuss it with the wider audience on dev list, probably someone else could help us with the resolution. Stepan?

        Show
        Ilya Okomin added a comment - Hello Stepan! Sorry for delay in my reply. I've checked your fix: 1. Suggested scenario works (however ../JavaSoft / Prefs key refreshed for me only when I had allowed Enumerate Subkeys and Query Value permissions to this key). This scenario worked only for the first time, when java/lang key subtree was created. If you are implementing the scenario when java/lang was already created - SecurityException is thrown on Harmony. 2. With only allowed 'Create Subkey' permission testSystemNodeForPackage passed for me( only for the first run, next runs failed with SecurityException ) on Harmony, but it failed on RI with exception: Could not open windows registry node Software\JavaSoft\Prefs\java\lang at root 0x80000002: Access denied java.lang.SecurityException: Could not open windows registry node Software\JavaSoft\Prefs\java\lang at root 0x80000002: Access denied at java.util.prefs.WindowsPreferences.openKey(WindowsPreferences.java:496) at java.util.prefs.WindowsPreferences.openKey(WindowsPreferences.java:463) at java.util.prefs.WindowsPreferences.openKey(WindowsPreferences.java:449) at java.util.prefs.WindowsPreferences.childrenNamesSpi(WindowsPreferences.java:717) at java.util.prefs.AbstractPreferences.childrenNames(AbstractPreferences.java:699) at org.apache.harmony.prefs.tests.java.util.prefs.PreferencesTest.testSystemNodeForPackage(PreferencesTest.java:74) at jrockit.reflect.VirtualNativeMethodInvoker.invoke(Ljava.lang.Object;[Ljava.lang.Object;)Ljava.lang.Object;(Unknown Source) 3. With combination of permissions, that I had on the machine for testing (I suppose Elena had the same permissions set): Allow permissions set: Query Value Enumerate Subkeys Notify Read Control with your fix testSystemNodeForPackage passed for me on RI, but it failed on Harmony with exception: java.lang.SecurityException at java.util.prefs.RegistryPreferencesImpl.childSpi(RegistryPreferencesImpl.java:116) at java.util.prefs.AbstractPreferences.getNodeFromBackend(AbstractPreferences.java:646) at java.util.prefs.AbstractPreferences.nodeImpl(AbstractPreferences.java:626) at java.util.prefs.AbstractPreferences.node(AbstractPreferences.java:597) at java.util.prefs.Preferences.systemNodeForPackage(Preferences.java:767) at org.apache.harmony.prefs.tests.java.util.prefs.PreferencesTest.testSystemNodeForPackage(PreferencesTest.java:62) at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:25) I want to add, despite test passed on RI there was a set of warnings printed to the log: [junit] WARNING: Could not create windows registry node Software\JavaSoft\Prefs\java at root 0x80000002. Windows RegCreateKeyEx(...) returned error code 5. [junit] 05.12.2006 16:10:22 java.util.prefs.WindowsPreferences WindowsRegOpenKey1 [junit] WARNING: Trying to recreate Windows registry node Software\JavaSoft\Prefs\java at root 0x80000002. [junit] 05.12.2006 16:10:22 java.util.prefs.WindowsPreferences openKey [junit] WARNING: Could not open windows registry node Software\JavaSoft\Prefs\java at root 0x80000002. Windows RegOpenKey(...) returned error code 2. [junit] Preferences p = System Preference Node: /java/lang ...etc... Summary: Test passes for users with allowed permissions. Spec keeps silence how to handle cases with certain users permissions and it clearly says that implementations are encouraged to throw SecurityException if the caller lacks sufficient privileges. Fix for this issue doesn't seem so easy to me. I would suggest to leave this as non-bug difference again or we can discuss it with the wider audience on dev list, probably someone else could help us with the resolution. Stepan?

          People

          • Assignee:
            Stepan Mishura
            Reporter:
            Elena Semukhina
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:

              Development