Re: RFR: 8181571: printing to CUPS fails on mac sandbox app [v3]

2021-08-24 Thread Alexander Scherbatiy
On Thu, 12 Aug 2021 19:34:44 GMT, Phil Race  wrote:

>> Alexander Scherbatiy has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Return null if printers are not found in sandboxed app on MacOS
>
> src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 96:
> 
>> 94: libFound = initIDs();
>> 95: if (libFound) {
>> 96:cupsServer = getCupsServer();
> 
> I think we should wrap all the new lines in isMac()
> Also can you explain the reasons for the logic that implies we may have a 
> server starting with "/"
> in which case your always change the cupServer to localhost even if it is not 
> sandboxed ?
> 
> I ask because it seems to contradict what you pasted
> "by the cupsServer() function and not changing the interface string to 
> "localhost""

The logic which replaces a server starting with "/" to localhost is the 
original logic which is implemented in native level in the getCupsServer() 
function:
https://github.com/openjdk/jdk/blob/f608e81ad8309a001b8a92563a93b8adee1ce2b8/src/java.desktop/unix/native/common/awt/CUPSfuncs.c#L176
The fix only moves this logic to the java level to store domainSocketPathname 
in case of sandboxed app on MacOS.

> src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 399:
> 
>> 397: return printerURIs;
>> 398: }
>> 399: }
> 
> So if getCupsDefaultPrinters() doesn't find anything you always continue to 
> the original code even though
> it would seem that you know we are in a sandboxed app and it won't find 
> anything ?

I updated the code to return null in case there are no printer names from  
j2d_cupsGetDests() function in a sandboxed on MacOS.

> src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 489:
> 
>> 487: return domainSocketPathname;
>> 488: }
>> 489: 
> 
> You will need to suppress deprecation warnings here.

Should I add `@SuppressWarnings("deprecation")` to the 
getDomainSocketPathname() method?
I see that CUPSPrinter class has `@SuppressWarnings("removal")` but its private 
method do not have any SuppressWarnings annotations.

> src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 506:
> 
>> 504: IPPPrintService.debug_println(debugPrefix+"libFound "+libFound);
>> 505: if (libFound) {
>> 506: String server = getDomainSocketPathname() != null ? 
>> getDomainSocketPathname() : getServer();
> 
> Split this long line

Updated.

> src/java.desktop/unix/native/common/awt/CUPSfuncs.c line 244:
> 
>> 242: DPRINTF("CUPSfuncs::bad alloc new array\n", "")
>> 243: (*env)->ExceptionClear(env);
>> 244: JNU_ThrowOutOfMemoryError(env, "OutOfMemoryError");
> 
> I find this weird. What is the ExceptionClear for ? The only possible 
> exception here is for
> an OOME which might be thrown by NewObjectArray. So you clear that and then 
> re-create it ?
> And who do will catch this ? What's the point ? Maybe just clear and return 
> null.

The array creation error handling is implemented in the similar way as it is 
done in the getMedia() function.
The ExceptionClear() has been added to the getMedia() by `8031001: [Parfait] 
warnings from b121 for jdk/src/solaris/native/sun/awt: JNI-related warnings`
I would prefer to have unified error handling in these methods. If 
ExceptionClear is not suitable in this place it would be better to recheck 
JDK-8031001 and update all places accordingly in a separate fix.

> src/java.desktop/unix/native/common/awt/CUPSfuncs.c line 253:
> 
>> 251: j2d_cupsFreeDests(num_dests, dests);
>> 252: DPRINTF("CUPSfuncs::bad alloc new string ->name\n", "")
>> 253: JNU_ThrowOutOfMemoryError(env, "OutOfMemoryError");
> 
> similar comments to above plus I am fairly sure you want to delete nameArray 
> since it isn't returned.
> For that matter if the NewString fails on the 4th printer you want to free 
> the 3 previous ones too before returning.

The code is updated to remove previously created strings and clear the 
nameArray in case of an error during new string creation.

-

PR: https://git.openjdk.java.net/jdk/pull/4861


Re: RFR: 8181571: printing to CUPS fails on mac sandbox app [v3]

2021-08-24 Thread Alexander Scherbatiy
> The issue is reproduced on macOS Big Sur 11.0.1 with jdk 16.0.1+9.
> 
> Create a native macOS app from the Hello.java file, sign and run it in 
> sandbox:
> 
> import javax.print.*;
> import javax.swing.*;
> 
> public class Hello {
> 
> public static void main(String[] args) throws Exception {
> SwingUtilities.invokeAndWait(() -> {
> boolean isSandboxed = System.getenv("APP_SANDBOX_CONTAINER_ID") 
> != null;
> PrintService defaultPrinter = 
> PrintServiceLookup.lookupDefaultPrintService();
> PrintService[] services = 
> PrintServiceLookup.lookupPrintServices(null, null);
> 
> StringBuilder builder = new StringBuilder();
> builder.append("is sandboxed: ").append(isSandboxed).append("\n");
> builder.append("default printer: 
> ").append(defaultPrinter).append("\n");
> int size = services.length;
> for (int i = 0; i < size; i++) {
> 
> builder.append("printer[").append(i).append("]=").append(services[i]).append("\n");
> }
> JOptionPane.showMessageDialog(null, builder.toString());
> });
> }
> }
> 
> The signed app in sandbox shows null default printer and 
> PrintServiceLookup.lookupPrintServices(null, null) returns "Unix Printer: lp".
> ![PrintSandboxedApp](https://bugs.openjdk.java.net/secure/attachment/95629/PrintSandboxedApp.png)
> 
> The problem has been discussed on  2d-dev mail list:
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-June/008375.html
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-July/008418.html
> 
> According to the discussion:
> 
>> I've submitted a DTS incident to Apple and a friend there has followed-up.
>> Their unofficial position is that java should be connecting to the cups 
>> interface returned
>> by the cupsServer() function and not changing the interface string to 
>> "localhost".
>> Security changes in 10.12.4 reject the TCP connection which they say confuses
>> network-client access with print access.  They don't seem interested in 
>> loosening that change.
> 
> 
> The proposed solution is to use the domain socket pathname in 
> httpConnect(...) cups function and cupsGetDests(...) to get list of printers 
> from cups  when the app is signed and is run in sandbox on MacOs.

Alexander Scherbatiy has updated the pull request incrementally with one 
additional commit since the last revision:

  Return null if printers are not found in sandboxed app on MacOS

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4861/files
  - new: https://git.openjdk.java.net/jdk/pull/4861/files/104e792b..067d0025

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4861=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4861=01-02

  Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4861.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4861/head:pull/4861

PR: https://git.openjdk.java.net/jdk/pull/4861


Re: RFR: 8181571: printing to CUPS fails on mac sandbox app [v2]

2021-08-24 Thread Alexander Scherbatiy
> The issue is reproduced on macOS Big Sur 11.0.1 with jdk 16.0.1+9.
> 
> Create a native macOS app from the Hello.java file, sign and run it in 
> sandbox:
> 
> import javax.print.*;
> import javax.swing.*;
> 
> public class Hello {
> 
> public static void main(String[] args) throws Exception {
> SwingUtilities.invokeAndWait(() -> {
> boolean isSandboxed = System.getenv("APP_SANDBOX_CONTAINER_ID") 
> != null;
> PrintService defaultPrinter = 
> PrintServiceLookup.lookupDefaultPrintService();
> PrintService[] services = 
> PrintServiceLookup.lookupPrintServices(null, null);
> 
> StringBuilder builder = new StringBuilder();
> builder.append("is sandboxed: ").append(isSandboxed).append("\n");
> builder.append("default printer: 
> ").append(defaultPrinter).append("\n");
> int size = services.length;
> for (int i = 0; i < size; i++) {
> 
> builder.append("printer[").append(i).append("]=").append(services[i]).append("\n");
> }
> JOptionPane.showMessageDialog(null, builder.toString());
> });
> }
> }
> 
> The signed app in sandbox shows null default printer and 
> PrintServiceLookup.lookupPrintServices(null, null) returns "Unix Printer: lp".
> ![PrintSandboxedApp](https://bugs.openjdk.java.net/secure/attachment/95629/PrintSandboxedApp.png)
> 
> The problem has been discussed on  2d-dev mail list:
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-June/008375.html
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-July/008418.html
> 
> According to the discussion:
> 
>> I've submitted a DTS incident to Apple and a friend there has followed-up.
>> Their unofficial position is that java should be connecting to the cups 
>> interface returned
>> by the cupsServer() function and not changing the interface string to 
>> "localhost".
>> Security changes in 10.12.4 reject the TCP connection which they say confuses
>> network-client access with print access.  They don't seem interested in 
>> loosening that change.
> 
> 
> The proposed solution is to use the domain socket pathname in 
> httpConnect(...) cups function and cupsGetDests(...) to get list of printers 
> from cups  when the app is signed and is run in sandbox on MacOs.

Alexander Scherbatiy has updated the pull request incrementally with two 
additional commits since the last revision:

 - Clean utf_str and nameArray references
 - Split long line in CUPSPrinter.isCupsRunning() method

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4861/files
  - new: https://git.openjdk.java.net/jdk/pull/4861/files/072cc498..104e792b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4861=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4861=00-01

  Stats: 11 lines in 2 files changed: 9 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4861.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4861/head:pull/4861

PR: https://git.openjdk.java.net/jdk/pull/4861


Re: RFR: 8181571: printing to CUPS fails on mac sandbox app

2021-08-12 Thread Phil Race
On Wed, 21 Jul 2021 15:45:55 GMT, Alexander Scherbatiy  
wrote:

> The issue is reproduced on macOS Big Sur 11.0.1 with jdk 16.0.1+9.
> 
> Create a native macOS app from the Hello.java file, sign and run it in 
> sandbox:
> 
> import javax.print.*;
> import javax.swing.*;
> 
> public class Hello {
> 
> public static void main(String[] args) throws Exception {
> SwingUtilities.invokeAndWait(() -> {
> boolean isSandboxed = System.getenv("APP_SANDBOX_CONTAINER_ID") 
> != null;
> PrintService defaultPrinter = 
> PrintServiceLookup.lookupDefaultPrintService();
> PrintService[] services = 
> PrintServiceLookup.lookupPrintServices(null, null);
> 
> StringBuilder builder = new StringBuilder();
> builder.append("is sandboxed: ").append(isSandboxed).append("\n");
> builder.append("default printer: 
> ").append(defaultPrinter).append("\n");
> int size = services.length;
> for (int i = 0; i < size; i++) {
> 
> builder.append("printer[").append(i).append("]=").append(services[i]).append("\n");
> }
> JOptionPane.showMessageDialog(null, builder.toString());
> });
> }
> }
> 
> The signed app in sandbox shows null default printer and 
> PrintServiceLookup.lookupPrintServices(null, null) returns "Unix Printer: lp".
> ![PrintSandboxedApp](https://bugs.openjdk.java.net/secure/attachment/95629/PrintSandboxedApp.png)
> 
> The problem has been discussed on  2d-dev mail list:
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-June/008375.html
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-July/008418.html
> 
> According to the discussion:
> 
>> I've submitted a DTS incident to Apple and a friend there has followed-up.
>> Their unofficial position is that java should be connecting to the cups 
>> interface returned
>> by the cupsServer() function and not changing the interface string to 
>> "localhost".
>> Security changes in 10.12.4 reject the TCP connection which they say confuses
>> network-client access with print access.  They don't seem interested in 
>> loosening that change.
> 
> 
> The proposed solution is to use the domain socket pathname in 
> httpConnect(...) cups function and cupsGetDests(...) to get list of printers 
> from cups  when the app is signed and is run in sandbox on MacOs.

Changes requested by prr (Reviewer).

src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 96:

> 94: libFound = initIDs();
> 95: if (libFound) {
> 96:cupsServer = getCupsServer();

I think we should wrap all the new lines in isMac()
Also can you explain the reasons for the logic that implies we may have a 
server starting with "/"
in which case your always change the cupServer to localhost even if it is not 
sandboxed ?

I ask because it seems to contradict what you pasted
"by the cupsServer() function and not changing the interface string to 
"localhost""

src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 399:

> 397: return printerURIs;
> 398: }
> 399: }

So if getCupsDefaultPrinters() doesn't find anything you always continue to the 
original code even though
it would seem that you know we are in a sandboxed app and it won't find 
anything ?

src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 489:

> 487: return domainSocketPathname;
> 488: }
> 489: 

You will need to suppress deprecation warnings here.

src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 506:

> 504: IPPPrintService.debug_println(debugPrefix+"libFound "+libFound);
> 505: if (libFound) {
> 506: String server = getDomainSocketPathname() != null ? 
> getDomainSocketPathname() : getServer();

Split this long line

src/java.desktop/unix/native/common/awt/CUPSfuncs.c line 244:

> 242: DPRINTF("CUPSfuncs::bad alloc new array\n", "")
> 243: (*env)->ExceptionClear(env);
> 244: JNU_ThrowOutOfMemoryError(env, "OutOfMemoryError");

I find this weird. What is the ExceptionClear for ? The only possible exception 
here is for
an OOME which might be thrown by NewObjectArray. So you clear that and then 
re-create it ?
And who do will catch this ? What's the point ? Maybe just clear and return 
null.

src/java.desktop/unix/native/common/awt/CUPSfuncs.c line 253:

> 251: j2d_cupsFreeDests(num_dests, dests);
> 252: DPRINTF("CUPSfuncs::bad alloc new string ->name\n", "")
> 253: JNU_ThrowOutOfMemoryError(env, "OutOfMemoryError");

similar comments to above plus I am fairly sure you want to delete nameArray 
since it isn't returned.
For that matter if the NewString fails on the 4th printer you want to free the 
3 previous ones too before returning.

-

PR: https://git.openjdk.java.net/jdk/pull/4861


Re: RFR: 8181571: printing to CUPS fails on mac sandbox app

2021-08-05 Thread Alexander Scherbatiy
On Wed, 28 Jul 2021 17:18:12 GMT, NikolayTach 
 wrote:

> [JDK-8247768 ](https://bugs.openjdk.java.net/browse/JDK-8247768)
> Same here played 6 times yet moved, 8 issues missed.

What is the relation between JDK-8247768 and the current pull request?
Should something be updated in the pull request description?

-

PR: https://git.openjdk.java.net/jdk/pull/4861


Re: RFR: 8181571: printing to CUPS fails on mac sandbox app

2021-07-28 Thread NikolayTach
On Wed, 21 Jul 2021 15:45:55 GMT, Alexander Scherbatiy  
wrote:

> The issue is reproduced on macOS Big Sur 11.0.1 with jdk 16.0.1+9.
> 
> Create a native macOS app from the Hello.java file, sign and run it in 
> sandbox:
> 
> import javax.print.*;
> import javax.swing.*;
> 
> public class Hello {
> 
> public static void main(String[] args) throws Exception {
> SwingUtilities.invokeAndWait(() -> {
> boolean isSandboxed = System.getenv("APP_SANDBOX_CONTAINER_ID") 
> != null;
> PrintService defaultPrinter = 
> PrintServiceLookup.lookupDefaultPrintService();
> PrintService[] services = 
> PrintServiceLookup.lookupPrintServices(null, null);
> 
> StringBuilder builder = new StringBuilder();
> builder.append("is sandboxed: ").append(isSandboxed).append("\n");
> builder.append("default printer: 
> ").append(defaultPrinter).append("\n");
> int size = services.length;
> for (int i = 0; i < size; i++) {
> 
> builder.append("printer[").append(i).append("]=").append(services[i]).append("\n");
> }
> JOptionPane.showMessageDialog(null, builder.toString());
> });
> }
> }
> 
> The signed app in sandbox shows null default printer and 
> PrintServiceLookup.lookupPrintServices(null, null) returns "Unix Printer: lp".
> ![PrintSandboxedApp](https://bugs.openjdk.java.net/secure/attachment/95629/PrintSandboxedApp.png)
> 
> The problem has been discussed on  2d-dev mail list:
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-June/008375.html
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-July/008418.html
> 
> According to the discussion:
> 
>> I've submitted a DTS incident to Apple and a friend there has followed-up.
>> Their unofficial position is that java should be connecting to the cups 
>> interface returned
>> by the cupsServer() function and not changing the interface string to 
>> "localhost".
>> Security changes in 10.12.4 reject the TCP connection which they say confuses
>> network-client access with print access.  They don't seem interested in 
>> loosening that change.
> 
> 
> The proposed solution is to use the domain socket pathname in 
> httpConnect(...) cups function and cupsGetDests(...) to get list of printers 
> from cups  when the app is signed and is run in sandbox on MacOs.

[JDK-8247768 ](https://bugs.openjdk.java.net/browse/JDK-8247768)
Same here played 6 times yet moved, 8 issues missed.

-

PR: https://git.openjdk.java.net/jdk/pull/4861