[OpenJDK 2D-Dev] RFR: 8181571: printing to CUPS fails on mac sandbox app [v3]

[Alexander Scherbatiy]

On Thu, 12 Aug 2021 19:34:44 GMT, Phil Race <prr at openjdk.org> 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