Adding constant for line.separator and friends

[martinrb]

(In response to Joe, who once asked me for little things to add
to the core libraries)

Lots of classes need to use System.getProperty("line.separator").
Many don't do it right because you need to use
a doPrivileged block whenever you read a system property.
Yet it is no secret - you can divine the line separator
even if you have no trust with the security manager.

Here's a strawman proposal:

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/line.separator/

diff --git a/src/share/classes/java/lang/System.java
b/src/share/classes/java/lang/System.java
--- a/src/share/classes/java/lang/System.java
+++ b/src/share/classes/java/lang/System.java
@@ -620,6 +620,32 @@
     }

     /**
+     * Defines some standard system properties as constant strings.
+     */
+    public static class StandardProperties {
+        public final static String FILE_SEPARATOR;
+        public final static String PATH_SEPARATOR;
+        public final static String LINE_SEPARATOR;
+
+        static {
+            String[] props =
+                AccessController.doPrivileged
+                (new PrivilegedAction<String[]>() {
+                    public String[] run() {
+                        return new String[] {
+                            getProperty("file.separator"),
+                            getProperty("path.separator"),
+                            getProperty("line.separator")
+                        };
+                    }
+                });
+            FILE_SEPARATOR = props[0];
+            PATH_SEPARATOR = props[1];
+            LINE_SEPARATOR = props[2];
+        }
+    }
+
+    /**
      * Sets the system properties to the <code>Properties</code>
      * argument.
      * <p>

[weijun.wang]

Really necessary? These three system properties are already allowed in  
the sandbox.

$ cat lib/security/java.policy
....
        permission java.util.PropertyPermission "file.separator", "read";
        permission java.util.PropertyPermission "path.separator", "read";
        permission java.util.PropertyPermission "line.separator", "read";
....

Max

On Nov 10, 2009, at 8:34 AM, Martin Buchholz wrote:

> (In response to Joe, who once asked me for little things to add
> to the core libraries)
>
> Lots of classes need to use System.getProperty("line.separator").
> Many don't do it right because you need to use
> a doPrivileged block whenever you read a system property.
> Yet it is no secret - you can divine the line separator
> even if you have no trust with the security manager.
>
> Here's a strawman proposal:
>
> http://cr.openjdk.java.net/~martin/webrevs/openjdk7/line.separator/
>
> diff --git a/src/share/classes/java/lang/System.java
> b/src/share/classes/java/lang/System.java
> --- a/src/share/classes/java/lang/System.java
> +++ b/src/share/classes/java/lang/System.java
> @@ -620,6 +620,32 @@
>     }
>
>     /**
> +     * Defines some standard system properties as constant strings.
> +     */
> +    public static class StandardProperties {
> +        public final static String FILE_SEPARATOR;
> +        public final static String PATH_SEPARATOR;
> +        public final static String LINE_SEPARATOR;
> +
> +        static {
> +            String[] props =
> +                AccessController.doPrivileged
> +                (new PrivilegedAction<String[]>() {
> +                    public String[] run() {
> +                        return new String[] {
> +                            getProperty("file.separator"),
> +                            getProperty("path.separator"),
> +                            getProperty("line.separator")
> +                        };
> +                    }
> +                });
> +            FILE_SEPARATOR = props[0];
> +            PATH_SEPARATOR = props[1];
> +            LINE_SEPARATOR = props[2];
> +        }
> +    }
> +
> +    /**
>      * Sets the system properties to the <code>Properties</code>
>      * argument.
>      * <p>

[martinrb]

On Mon, Nov 9, 2009 at 16:34, Martin Buchholz <martinrb@...> wrote:
> (In response to Joe, who once asked me for little things to add
> to the core libraries)
>
> Lots of classes need to use System.getProperty("line.separator").
> Many don't do it right because you need to use
> a doPrivileged block whenever you read a system property.
> Yet it is no secret - you can divine the line separator
> even if you have no trust with the security manager.

Let me clarify somewhat.
The default Policy
defined in src/share/lib/security/java.policy
allows you to access standard system properties
such as line.separator, but...

$ cat Println.java &&  java Println
import java.security.*;

public class Println {
    public static void main(String[] args) {
        Policy.setPolicy(new java.security.Policy() {
                public boolean implies(ProtectionDomain pd, Permission p) {
                    return false;
                }});
        System.setSecurityManager(new SecurityManager());
        System.out.printf("%n");
        String.format("%n");
    }
}
==> javac -source 1.6 -Xlint:all Println.java
==> java -esa -ea Println
Exception in thread "main" java.security.AccessControlException:
access denied (java.util.PropertyPermission line.separator read)
        at java.security.AccessControlContext.checkPermission(AccessControlContext.java:342)
        at java.security.AccessController.checkPermission(AccessController.java:553)
        at java.lang.SecurityManager.checkPermission(SecurityManager.java:549)
        at java.lang.SecurityManager.checkPropertyAccess(SecurityManager.java:1302)
        at java.lang.System.getProperty(System.java:669)
        at java.util.Formatter$FormatSpecifier.print(Formatter.java:2700)
        at java.util.Formatter.format(Formatter.java:2437)
        at java.io.PrintStream.format(PrintStream.java:937)
        at java.io.PrintStream.printf(PrintStream.java:838)
        at Println.main(Println.java:10)

---

Also, File.java only makes available the first char of the system properties
(ensuring that Java will never work on platforms where
file.separator is more than one character,
or is a supplementary character?

Martin

[martinrb]

More lobbying....

Here's how many source files in the JDK
access this system property.

$ rg '"line.separator"' | wc -l
43

Some use doPrivileged, some don't.
Some cache statically, some cache in an object,
some don't cache at all.

Martin

[Joseph D. Darcy]

Martin Buchholz wrote:

> (In response to Joe, who once asked me for little things to add
> to the core libraries)
>
> Lots of classes need to use System.getProperty("line.separator").
> Many don't do it right because you need to use
> a doPrivileged block whenever you read a system property.
> Yet it is no secret - you can divine the line separator
> even if you have no trust with the security manager.
>
> Here's a strawman proposal:
>
> http://cr.openjdk.java.net/~martin/webrevs/openjdk7/line.separator/
>
> diff --git a/src/share/classes/java/lang/System.java
> b/src/share/classes/java/lang/System.java
> --- a/src/share/classes/java/lang/System.java
> +++ b/src/share/classes/java/lang/System.java
> @@ -620,6 +620,32 @@
>      }
>
>      /**
> +     * Defines some standard system properties as constant strings.
> +     */
> +    public static class StandardProperties {
> +        public final static String FILE_SEPARATOR;
> +        public final static String PATH_SEPARATOR;
> +        public final static String LINE_SEPARATOR;
> +
> +        static {
> +            String[] props =
> +                AccessController.doPrivileged
> +                (new PrivilegedAction<String[]>() {
> +                    public String[] run() {
> +                        return new String[] {
> +                            getProperty("file.separator"),
> +                            getProperty("path.separator"),
> +                            getProperty("line.separator")
> +                        };
> +                    }
> +                });
> +            FILE_SEPARATOR = props[0];
> +            PATH_SEPARATOR = props[1];
> +            LINE_SEPARATOR = props[2];
> +        }
> +    }
> +
> +    /**
>       * Sets the system properties to the <code>Properties</code>
>       * argument.
>       * <p>
>  

Hi Martin,

Given that these values are not true constants since they vary across
platforms, I think it is misleading to make them look like constant by
having them be "public static final" fields with ALL CAPS names.

I would prefer to see these values returned by wrapper methods that did
the necessary security checks and caching.

-Joe

[martinrb]

On Mon, Nov 9, 2009 at 20:14, Joe Darcy <Joe.Darcy@...> wrote:

Martin Buchholz wrote:

Hi Martin,

Given that these values are not true constants since they vary across platforms, I think it is misleading to make them look like constant by having them be "public static final" fields with ALL CAPS names.

I would prefer to see these values returned by wrapper methods that did the necessary security checks and caching.

We could certainly turn the fields into methods.

Since line.separator is the most popular system property,
and other system properties like
java.home have vague security implications
that would suggest they must be protected by
a security manager, we might consider just
making the one system property available.
Perhaps very simply

String System.lineSeparator()?

or we could put it into java.io.File

String File.lineSeparator()
 
Martin

-Joe

[Mark Reinhold]

> Date: Mon, 09 Nov 2009 20:29:51 -0800
> From: Martin Buchholz <martinrb@...>

> ...
>
> Since line.separator is the most popular system property,
> and other system properties like
> java.home have vague security implications
> that would suggest they must be protected by
> a security manager, we might consider just
> making the one system property available.
> Perhaps very simply
>
> String System.lineSeparator()?

This seems like a much better solution than adding a whole static inner
class to java.lang.System.

Yet ... how much of a problem is this outside of the JDK itself and,
more generally, for new code rather than old?

If there are n (for n <= 43) problematic uses in the JDK then we could
just fix those without defining any new public API.  For new code we
should encourage people to use String.format("...%n...") rather than
access the line.separator property explicitly.

- Mark

[martinrb]

On Mon, Nov 9, 2009 at 21:54, Mark Reinhold <mr@...> wrote:

> Date: Mon, 09 Nov 2009 20:29:51 -0800
> From: Martin Buchholz <martinrb@...>

> ...

>
> Since line.separator is the most popular system property,
> and other system properties like
> java.home have vague security implications
> that would suggest they must be protected by
> a security manager, we might consider just
> making the one system property available.
> Perhaps very simply
>
> String System.lineSeparator()?

This seems like a much better solution than adding a whole static inner
class to java.lang.System.

Yet ... how much of a problem is this outside of the JDK itself and,
more generally, for new code rather than old?

If there are n (for n <= 43) problematic uses in the JDK then we could
just fix those without defining any new public API.  For new code we
should encourage people to use String.format("...%n...") rather than
access the line.separator property explicitly.

Hacking on Formatter is how I got here.

String.format("%n")" is both at risk of throwing SecurityException
(because Formatter does not use doPrivileged;
my proposal here is a step to fixing that)
and is rather slow.
Too slow for performance-sensitive uses.

Martin

[Remi Forax]

Le 10/11/2009 08:07, Martin Buchholz a écrit :

On Mon, Nov 9, 2009 at 21:54, Mark Reinhold <mr@...> wrote:

> Date: Mon, 09 Nov 2009 20:29:51 -0800
> From: Martin Buchholz <martinrb@...>

> ...

>
> Since line.separator is the most popular system property,
> and other system properties like
> java.home have vague security implications
> that would suggest they must be protected by
> a security manager, we might consider just
> making the one system property available.
> Perhaps very simply
>
> String System.lineSeparator()?

This seems like a much better solution than adding a whole static inner
class to java.lang.System.

Yet ... how much of a problem is this outside of the JDK itself and,
more generally, for new code rather than old?

If there are n (for n <= 43) problematic uses in the JDK then we could
just fix those without defining any new public API.  For new code we
should encourage people to use String.format("...%n...") rather than
access the line.separator property explicitly.

Hacking on Formatter is how I got here.

String.format("%n")" is both at risk of throwing SecurityException
(because Formatter does not use doPrivileged;
my proposal here is a step to fixing that)
and is rather slow.
Too slow for performance-sensitive uses.

Martin

Yes, printf is awfully slow.
I am in favor of something like that:

public class System {
  public String lineSeparator() {
    return StandardProperties.LINE_SEPARATOR;
  }
}

With StandardProperties private and LINE_SEPARATOR with a default visibility.

Rémi

[Stephen Colebourne-2]

2009/11/10 Mark Reinhold <mr@...>:
> Yet ... how much of a problem is this outside of the JDK itself and,
> more generally, for new code rather than old?
>
> If there are n (for n <= 43) problematic uses in the JDK then we could
> just fix those without defining any new public API.  For new code we
> should encourage people to use String.format("...%n...") rather than
> access the line.separator property explicitly.

Commons Lang defines the class SystemUtils:
http://commons.apache.org/lang/api-2.3/org/apache/commons/lang/SystemUtils.html
This has constants for all the main system properties.

Within my work 1.4MLOC codebase, SystemUtils.LINE_SEPARATOR is used 65
times, with a further 7 cases within other parts of Commons Lang
itself. This is a commonly used value.

SystemUtils contains many more constants though. This is beneficial
for code clarity, and avoiding "magic" property names that are
difficult to find when you want them. While most are rarely used, all
will be used occasionally (otherwise why make them available). Asking
developers to use "magic" names for the well-defined common cases
seems non-sensical.

As such I'm definitely in favour of line separator, but would also
want to see others (including common directory locations like IO
home).

Stephen

[Alex Miller-6]

I've seen multiple instances of constants for line.separator, etc in just about every application code base I've ever used.  I'd heartily vote for standardizing a well-defined way to get this in the JDK.  I think methods would be fine and System seems like the best location for them.

Alex

[martinrb]

On Tue, Nov 10, 2009 at 03:05, Stephen Colebourne <scolebourne@...> wrote:

2009/11/10 Mark Reinhold <mr@...>:

> Yet ... how much of a problem is this outside of the JDK itself and,
> more generally, for new code rather than old?
>
> If there are n (for n <= 43) problematic uses in the JDK then we could
> just fix those without defining any new public API.  For new code we
> should encourage people to use String.format("...%n...") rather than
> access the line.separator property explicitly.

Commons Lang defines the class SystemUtils:
http://commons.apache.org/lang/api-2.3/org/apache/commons/lang/SystemUtils.html
This has constants for all the main system properties.

Within my work 1.4MLOC codebase, SystemUtils.LINE_SEPARATOR is used 65
times, with a further 7 cases within other parts of Commons Lang
itself. This is a commonly used value.

SystemUtils contains many more constants though. This is beneficial
for code clarity, and avoiding "magic" property names that are
difficult to find when you want them. While most are rarely used, all
will be used occasionally (otherwise why make them available). Asking
developers to use "magic" names for the well-defined common cases
seems non-sensical.

As such I'm definitely in favour of line separator, but would also
want to see others (including common directory locations like IO
home).

I agree - I would also like to see a class that provides cheap and
compile-time-checked access to standard system properties,
like SystemUtils, but also guaranteeing to use doPrivileged
to avoid security manager problems.  But I don't think
this would be accepted in core jdk,
since some of the system properties like
user.name have privacy/security implications,
which someone must surely be depending on.

System.lineSeparator() is a small step,
but it's achievable.

 Martin

[Mark Reinhold]

> Date: Mon, 09 Nov 2009 23:07:05 -0800
> From: Martin Buchholz <martinrb@...>

> On Mon, Nov 9, 2009 at 21:54, Mark Reinhold <mr@...> wrote:
>> ...
>> Yet ... how much of a problem is this outside of the JDK itself and,
>> more generally, for new code rather than old?
>>
>> If there are n (for n <= 43) problematic uses in the JDK then we could
>> just fix those without defining any new public API.  For new code we
>> should encourage people to use String.format("...%n...") rather than
>> access the line.separator property explicitly.
>
> Hacking on Formatter is how I got here.
>
> String.format("%n")" is both at risk of throwing SecurityException
> (because Formatter does not use doPrivileged;
> my proposal here is a step to fixing that)
> and is rather slow.
> Too slow for performance-sensitive uses.

Okay, fair enough.

- Mark

[Ulf Zibis-2]

Am 10.11.2009 18:30, Martin Buchholz schrieb:
>
> System.lineSeparator() is a small step,
> but it's achievable.

My 2 cents:

I would like to see it in class java.io.File, as those properties occur
mainly in text files.
Also system property for encoding is named "file.encoding".

-Ulf

[Joseph D. Darcy]

Mark Reinhold wrote:

>> Date: Mon, 09 Nov 2009 23:07:05 -0800
>> From: Martin Buchholz <martinrb@...>
>>    
>
>  
>> On Mon, Nov 9, 2009 at 21:54, Mark Reinhold <mr@...> wrote:
>>    
>>> ...
>>> Yet ... how much of a problem is this outside of the JDK itself and,
>>> more generally, for new code rather than old?
>>>
>>> If there are n (for n <= 43) problematic uses in the JDK then we could
>>> just fix those without defining any new public API.  For new code we
>>> should encourage people to use String.format("...%n...") rather than
>>> access the line.separator property explicitly.
>>>      
>> Hacking on Formatter is how I got here.
>>
>> String.format("%n")" is both at risk of throwing SecurityException
>> (because Formatter does not use doPrivileged;
>> my proposal here is a step to fixing that)
>> and is rather slow.
>> Too slow for performance-sensitive uses.
>>    
>
> Okay, fair enough.
>
>  

Martin,

If we're settled on a method in System, please send a revised spec and
I'll file the ccc paperwork.

-Joe

[martinrb]

On Tue, Nov 10, 2009 at 10:06, Joseph D. Darcy <Joe.Darcy@...> wrote:
> Martin,
>
> If we're settled on a method in System, please send a revised spec and I'll
> file the ccc paperwork.

I'm agnostic about whether it belongs in System or File or elsewhere,
but my "serious" webrev below puts it in System.

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/line.separator/

Please file bug + ccc.

This also fixes a warning in System.java,
makes printf a little faster, and fixes the SecurityException
in the java program below:

import java.security.*;

public class Println {
   public static void main(String[] args) {
       Policy.setPolicy(new java.security.Policy() {
               public boolean implies(ProtectionDomain pd, Permission p) {
                   return false;
               }});
       System.setSecurityManager(new SecurityManager());
       System.out.printf("%n");
       String.format("%n");
   }
}

[Joseph D. Darcy]

On 11/10/09 04:40 PM, Martin Buchholz wrote:

On Tue, Nov 10, 2009 at 10:06, Joseph D. Darcy Joe.Darcy@... wrote:
  

Martin,

If we're settled on a method in System, please send a revised spec and I'll
file the ccc paperwork.
    

I'm agnostic about whether it belongs in System or File or elsewhere,
but my "serious" webrev below puts it in System.

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/line.separator/

Please file bug + ccc.

  

6900043 Add method to return line.separator property

-Joe

[Ulf Zibis-2]

Am 11.11.2009 01:40, Martin Buchholz schrieb:

> On Tue, Nov 10, 2009 at 10:06, Joseph D. Darcy <Joe.Darcy@...> wrote:
>  
>> Martin,
>>
>> If we're settled on a method in System, please send a revised spec and I'll
>> file the ccc paperwork.
>>    
>
> I'm agnostic about whether it belongs in System or File or elsewhere,
>  

At least, IMO, fileSeparator() and pathSeparator() would belong to
java.io.File or java.nio.file.FileSystem.

-Ulf

[martinrb]

On Wed, Nov 11, 2009 at 10:10, Ulf Zibis <Ulf.Zibis@...> wrote:

At least, IMO, fileSeparator() and pathSeparator() would belong to java.io.File or java.nio.file.FileSystem.

Those would have been better names,
but at this point they are not worth adding.

Martin
 

[Stephen Colebourne-2]

Martin Buchholz wrote:
> On Wed, Nov 11, 2009 at 10:10, Ulf Zibis <Ulf.Zibis@...
> <mailto:Ulf.Zibis@...>> wrote:
>     At least, IMO, fileSeparator() and pathSeparator() would belong to
>     java.io.File or java.nio.file.FileSystem.
> Those would have been better names,
> but at this point they are not worth adding.

I don't follow - why are they not worth adding now? Becuase they should
have been in JDK 1.0? Because JDK 7 is about to release? Some other
reason? How and who is deciding?

Over the past few weeks I've been tempted to ask for sponsorship of more
than one idea, change, or useful method (there are lots we could add).
But the process for what is accepted and what isn't seems very
arbitrary. As such, I'm just not bothering right now (nor am I working
on JSR-310, as the same arbitrary rejection could apply to that to).

Stephen