[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [10] RFR JDK-8186987:NullPointerException in RasterPrinterJob without PrinterRe
From: Philip Race <philip.race () oracle ! com>
Date: 2017-09-20 6:47:12
Message-ID: 59C20EF0.1060404 () oracle ! com
[Download RAW message or body]
Since we are relying on 0 then we need to be sure that the memory
starts off as zero so a non-zero value is not random left-over from
a previous user of that memory.
If I read correctly we allocate this memory using GlobalAlloc in
AwtPrintControl::getDevmode
and there pass GPTR which should ensure it is zeroed per
https://msdn.microsoft.com/en-us/library/windows/desktop/aa366574(v=vs.85).aspx
So we should be OK .. and +1 to this version but I find this driver
behaviour odd ..
-phil.
On 9/19/17, 11:29 PM, Prasanta Sadhukhan wrote:
>
> Hi Phil,
>
>
> On 9/19/2017 3:16 AM, Phil Race wrote:
> > Hi,
> >
> >
> > On 09/04/2017 03:07 AM, Prasanta Sadhukhan wrote:
> > > Hi Prahalad, Phil,
> > >
> > > JDK uses DocumentProperties function [1] to query the printer
> > > configuration data in windows.
> > >
> > > For a RIcoh printer which we (in IDC) use by default,
> > > pDevMode->dmFields is initialized to 0x200ff0f, which corresponds to
> > > DM_MEDIATYPE | DM_DUPLEX | DM_YRESOLUTION | DM_TTOPTION |
> > > DM_COLLATE | DM_COPIES | DM_DEFAULTSOURCE | DM_PRINTQUALITY |
> > > DM_COLOR | DM_PAPERWIDTH | DM_PAPERLENGTH | DM_PAPERSIZE |
> > > DM_ORIENTATION
> > >
> > > But, for, Brother printer, pDevMode->dmFields is initialized to
> > > 0x1930f which corresponds to
> > > DM_FORMNAME | DM_DUPLEX | DM_COLLATE | DM_COPIES |
> > > DM_DEFAULTSOURCE | DM_PAPERWIDTH | DM_PAPERLENGTH | DM_PAPERSIZE |
> > > DM_ORIENTATION
> > > so there is no DM_YRESOLUTION and DM_PRINTQUALITY in dmFields,
> > > so even though YRESOLUTION and PRINTQUALITY is populated by
> > > > DocumentProperties API, the corresponding indices are not set,
> > > resulting in having "GETDEFAULT_ERROR" [-50] in the array for those
> > > indices.
> > >
> >
> > Very odd. It sounds like a driver bug and I'm surprised that the
> > Microsoft HWQL tests don't catch it.
> > Was the driver directly from Brother ?
> Yes, I took the driver from
> http://support.brother.com/g/b/downloadlist.aspx?c=us_ot&lang=en&prod=hl2240d_all&os=93
>
> > Perhaps someone considered those fields so mandatory
> > that you don't even need to check that they are set ?
> >
> > This case reminds me of the fishy settings for a different bug
> > reviewed here :
> > http://mail.openjdk.java.net/pipermail/2d-dev/2016-June/007011.html
> > so ... I suppose this might be OK but still ..
> >
> > > I have modified my fix to populate the yresolution and printquality
> > > indices even though dmFields are not set by :DocumeProperties,
> > > provided those fields are not 0.
> >
> >
> > Actually the code says you are doing that only if as they are GREATER
> > than zero .. not just different than zero.
> >
> > So here
> > 919 if (pDevMode->dmFields& DM_PRINTQUALITY || pDevMode->dmPrintQuality> 0) {
> >
> > If quality is used by a driver but it forget to set the mask bit then you will \
> > still ignore it. Perhaps you want != 0 ?
> Yes, right. I actually overlooked the spec which says
>
> *dmPrintQuality*
>
> Specifies the printer resolution. There are four predefined
> device-independent values:
>
> *DMRES_HIGH*
> *DMRES_MEDIUM*
> *DMRES_LOW*
> *DMRES_DRAFT*
>
> If a positive value is specified, it specifies the number of dots
> per inch (DPI) and is therefore device dependent
>
> and as per wingdi.h, these values are -ve. I have updated the fix to
> use !=0.
>
> >
> >
> > >
> > > I have retained defaulting to low 300 dpi resolution as there might
> > > be a case when AwtPrintControl::getDevmode() fails resulting in
> > > returning default values which is -50.
> >
> > Are you referring to the code in Win32PrintService.java ?
> > I'm not sure about doing that. The way I read your code is now if a
> > driver specifies dmQuality as negative
> > and leaves yRes at zero (meaning ignore it) .. you now no longer
> > ignore it and return a new PrinterResolution
> > for a made up (300,300) resolution.
> >
> Ok, right. I looked at the spec and saw what you are saying
>
> *dmYResolution*
>
> Specifies the y-resolution, in dots per inch, of the printer. If
> the printer initializes this member, the *dmPrintQuality* member
> specifies the x-resolution
>
> which means, if dmPrintQuality is initialized +ve for x resolution,
> the y-resolution is specified by dmYResolution.
> Or in reverse way, if dmPrintQuality is -ve, then there might not be
> any need of specifying dmYResolution.
>
> I have updated the webrev to cater to both changes
>
> http://cr.openjdk.java.net/~psadhukhan/8186987/webrev.02/
>
> Regards
> Prasanta
>
>
> > > Also, in linux, we do the similar
> > > http://hg.openjdk.java.net/jdk10/client/jdk/file/70359afda5d0/src/java.desktop/unix/classes/sun/print/IPPPrintService.java#l1565
> > >
> > What we do in Linux (for Postscript) is different as postscript
> > allows you to specify the DPI for your content
> > and the interpreter must handle that.
> >
> > -phil.
> >
> > > http://cr.openjdk.java.net/~psadhukhan/8186987/webrev.01/
> > >
> > > Regards
> > > Prasanta
> > > [1]
> > > [https://msdn.microsoft.com/en-us/library/windows/desktop/dd183576(v=vs.85).aspx] \
> > >
> > > On 9/4/2017 8:28 AM, Prahalad Kumar Narayanan wrote
> > > > Hello Prasanta
> > > >
> > > > Thanks for the explanation.
> > > > Being new to the Printing subsystem, it helped get the context of
> > > > the problem.
> > > >
> > > > As I understand, the problem is due to getDefaultPrinterSetings()
> > > > returning negative values for defYRes and quality.
> > > > And the fix evades such negative values with a hardcoded low 300
> > > > DPI resolution.
> > > >
> > > > I 'm suspecting the bug to be in the code that returns values for
> > > > default printer settings
> > > > . Since these values are device specific, I believe, the code might
> > > > use few platform APIs to query the resolutions on that printer
> > > > device & return the same.
> > > > . It's here that default resolutions are not retrieved properly.
> > > > . So my view is to trace this location & fix the population of
> > > > default printer settings than a hardcoded DPI resolution.
> > > > . When a problem has surfaced on one printer, there is
> > > > possibility for the same to occur on many devices as well.
> > > > . Besides printers may not be supporting low 300 DPI
> > > > resolution going forward.
> > > >
> > > > I may be wrong in my understanding. You could wait for other's
> > > > review & follow up.
> > > >
> > > > Thank you
> > > > Have a good day
> > > >
> > > > Prahalad N.
> > > >
> > > > -----Original Message-----
> > > > From: Prasanta Sadhukhan
> > > > Sent: Thursday, August 31, 2017 3:39 PM
> > > > To: Philip Race; 2d-dev
> > > > Subject: [OpenJDK 2D-Dev] [10] RFR JDK-8186987:NullPointerException
> > > > in RasterPrinterJob without PrinterResolution
> > > >
> > > > Hi All,
> > > >
> > > > Please review a fix for an issue where it a NPE is seen when an
> > > > attempt is made to print to Brother HL-2240D series printer.
> > > >
> > > > It seems when RasterPrinterJob#setAttributes() is called with no
> > > > PrinterResolution attribute set, it first checks if
> > > > PrinterResolution category is supported.
> > > > If it is supported, then it sees if the supplied resolution value
> > > > is supported. Now, since no PrinterResolution attribute is set, so
> > > > isSupportedValue() returns false [as "printer resolution attribute"
> > > > object is null]
> > > > It then goes to get the default resolution attribute via
> > > > getDefaultAttributeValue() which calls getDefaultPrinterSettings()
> > > > and use yRes,Quality from this printer to construct a
> > > > "PrinterResolution"
> > > > object.
> > > >
> > > > Now, it seems in Brother HL-2240D series printer, it supports 3
> > > > resolution [300, 600, HQ 1200] but for all these 3 resolutions,
> > > > getDefaultPrinterSettings() returns -50 for yRes and Quality.
> > > > So, as per this code
> > > > http://hg.openjdk.java.net/jdk10/client/jdk/file/dbb5b171a16b/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1189 \
> > > >
> > > > res < 0 and no PrinterResolution object is instantiated so when RPJ
> > > > accesses
> > > > printerResAttr.getCrossFeedResolution(ResolutionSyntax.DPI); it
> > > > causes NPE.
> > > >
> > > > Proposed fix is to create a default lowly 300 dpi PrinterResolution
> > > > if, for some reason, yRes and Quality from printer comes out -ve.
> > > > http://cr.openjdk.java.net/~psadhukhan/8186987/webrev.00/
> > > >
> > > > Regards
> > > > Prasanta
> > >
> >
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
Since we are relying on 0 then we need to be sure that the memory<br>
starts off as zero so a non-zero value is not random left-over from<br>
a previous user of that memory.<br>
<br>
If I read correctly we allocate this memory using GlobalAlloc in
AwtPrintControl::getDevmode<br>
and there pass GPTR which should ensure it is zeroed per<br>
<a class="moz-txt-link-freetext" \
href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa366574(v=vs.85).aspx" \
>https://msdn.microsoft.com/en-us/library/windows/desktop/aa366574(v=vs.85).aspx</a><br>
>
<br>
So we should be OK .. and +1 to this version but I find this driver
behaviour odd ..<br>
<br>
-phil.<br>
<br>
On 9/19/17, 11:29 PM, Prasanta Sadhukhan wrote:
<blockquote
cite="mid:67a512e5-a13f-fef0-cf28-bd5e20f7e34f@oracle.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<p>Hi Phil,<br>
</p>
<br>
<div class="moz-cite-prefix">On 9/19/2017 3:16 AM, Phil Race
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:c1461e47-33f5-4bae-653b-4240ee668562@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
Hi,<br>
<br>
<br>
<div class="moz-cite-prefix">On 09/04/2017 03:07 AM, Prasanta
Sadhukhan wrote:<br>
</div>
<blockquote type="cite"
cite="mid:2cdbb171-6a58-641d-5865-891966ce156f@oracle.com">Hi
Prahalad, Phil, <br>
<br>
JDK uses DocumentProperties function [1] to query the printer
configuration data in windows. <br>
<br>
For a RIcoh printer which we (in IDC) use by default,
pDevMode->dmFields is initialized to 0x200ff0f, which
corresponds to <br>
DM_MEDIATYPE | DM_DUPLEX | DM_YRESOLUTION | DM_TTOPTION |
DM_COLLATE | DM_COPIES | DM_DEFAULTSOURCE |
DM_PRINTQUALITY | <br>
DM_COLOR | DM_PAPERWIDTH | DM_PAPERLENGTH | DM_PAPERSIZE |
DM_ORIENTATION <br>
<br>
But, for, Brother printer, pDevMode->dmFields is
initialized to 0x1930f which corresponds to <br>
DM_FORMNAME | DM_DUPLEX | DM_COLLATE | DM_COPIES |
DM_DEFAULTSOURCE | DM_PAPERWIDTH | DM_PAPERLENGTH |
DM_PAPERSIZE | DM_ORIENTATION <br>
so there is no DM_YRESOLUTION and DM_PRINTQUALITY in dmFields,
<br>
so even though YRESOLUTION and PRINTQUALITY is populated by
:DocumentProperties API, the corresponding indices are not
set, <br>
resulting in having "GETDEFAULT_ERROR" [-50] in the array for
those indices. <br>
<br>
</blockquote>
<br>
Very odd. It sounds like a driver bug and I'm surprised that the
Microsoft HWQL tests don't catch it.<br>
Was the driver directly from Brother ? </blockquote>
Yes, I took the driver from
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://support.brother.com/g/b/downloadlist.aspx?c=us_ot&lang=en&prod=hl \
2240d_all&os=93">http://support.brother.com/g/b/downloadlist.aspx?c=us_ot&lang=en&prod=hl2240d_all&os=93</a><br>
<br>
<blockquote type="cite"
cite="mid:c1461e47-33f5-4bae-653b-4240ee668562@oracle.com">Perhaps
someone considered those fields so mandatory<br>
that you don't even need to check that they are set ?<br>
<br>
This case reminds me of the fishy settings for a different bug
reviewed here : <a class="moz-txt-link-freetext"
href="http://mail.openjdk.java.net/pipermail/2d-dev/2016-June/007011.html"
moz-do-not-send="true">http://mail.openjdk.java.net/pipermail/2d-dev/2016-June/007011.html</a>
so ... I suppose this might be OK but still ..<br>
<br>
<blockquote type="cite"
cite="mid:2cdbb171-6a58-641d-5865-891966ce156f@oracle.com">I
have modified my fix to populate the yresolution and
printquality indices even though dmFields are not set by
:DocumeProperties, provided those fields are not 0. <br>
</blockquote>
<br>
<br>
Actually the code says you are doing that only if as they are
GREATER than zero .. not just different than zero.<br>
<br>
So here<br>
<pre><span class="changed"> 919 if (pDevMode->dmFields & \
DM_PRINTQUALITY || pDevMode->dmPrintQuality > 0) {
If quality is used by a driver but it forget to set the mask bit then you will still \
ignore it. Perhaps you want != 0 ?
</span></pre>
</blockquote>
Yes, right. I actually overlooked the spec which says<br>
<dl>
<dt><strong>dmPrintQuality</strong></dt>
<dd>
<p> Specifies the printer resolution. There are four
predefined device-independent values:</p>
<dl>
<dt><strong class="">DMRES_HIGH</strong></dt>
<dt><strong>DMRES_MEDIUM</strong></dt>
<dt><strong>DMRES_LOW</strong></dt>
<dt><strong>DMRES_DRAFT</strong></dt>
</dl>
<p class="">If a positive value is specified, it specifies the
number of dots per inch (DPI) and is therefore device
dependent</p>
</dd>
<dt>and as per wingdi.h, these values are -ve. I have updated
the fix to use !=0.<br>
</dt>
</dl>
<blockquote type="cite"
cite="mid:c1461e47-33f5-4bae-653b-4240ee668562@oracle.com">
<pre><span class="changed">
</span></pre>
<blockquote type="cite"
cite="mid:2cdbb171-6a58-641d-5865-891966ce156f@oracle.com"> <br>
I have retained defaulting to low 300 dpi resolution as there
might be a case when AwtPrintControl::getDevmode() fails
resulting in returning default values which is -50. <br>
</blockquote>
<br>
Are you referring to the code in Win32PrintService.java ?<br>
I'm not sure about doing that. The way I read your code is now
if a driver specifies dmQuality as negative<br>
and leaves yRes at zero (meaning ignore it) .. you now no longer
ignore it and return a new PrinterResolution<br>
for a made up (300,300) resolution.<br>
<br>
</blockquote>
Ok, right. I looked at the spec and saw what you are saying<br>
<dl>
<dt><strong class="">dmYResolution</strong></dt>
<dd>
<p> Specifies the y-resolution, in dots per inch, of the
printer. If the printer initializes this member, the \
<strong>dmPrintQuality</strong> member specifies the x-resolution</p>
</dd>
<dt>which means, if dmPrintQuality is initialized +ve for x
resolution, the y-resolution is specified by dmYResolution. <br>
</dt>
<dt>Or in reverse way, if dmPrintQuality is -ve, then there
might not be any need of specifying dmYResolution.</dt>
</dl>
<p>I have updated the webrev to cater to both changes</p>
<p><a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/8186987/webrev.02/">http://cr.openjdk.java.net/~psadhukhan/8186987/webrev.02/</a></p>
Regards<br>
Prasanta<br>
<dl>
<dd><br>
</dd>
</dl>
<blockquote type="cite"
cite="mid:c1461e47-33f5-4bae-653b-4240ee668562@oracle.com">
<blockquote type="cite"
cite="mid:2cdbb171-6a58-641d-5865-891966ce156f@oracle.com">Also,
in linux, we do the similar <a class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk10/client/jdk/file/70359afda5d0/src/java.desktop/unix/classes/sun/print/IPPPrintService.java#l1565"
moz-do-not-send="true">http://hg.openjdk.java.net/jdk10/client/jdk/file/70359afda5d0/src/java.desktop/unix/classes/sun/print/IPPPrintService.java#l1565</a><br>
<br>
</blockquote>
What we do in Linux (for Postscript) is different as postscript
allows you to specify the DPI for your content<br>
and the interpreter must handle that.<br>
<br>
-phil.<br>
<br>
<blockquote type="cite"
cite="mid:2cdbb171-6a58-641d-5865-891966ce156f@oracle.com"><a
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/8186987/webrev.01/"
moz-do-not-send="true">http://cr.openjdk.java.net/~psadhukhan/8186987/webrev.01/</a>
<br>
<br>
Regards <br>
Prasanta <br>
[1] [<a class="moz-txt-link-freetext"
href="https://msdn.microsoft.com/en-us/library/windows/desktop/dd183576%28v=vs.85%29.aspx"
moz-do-not-send="true">https://msdn.microsoft.com/en-us/library/windows/desktop/dd183576(v=vs.85).aspx</a>]
<br>
On 9/4/2017 8:28 AM, Prahalad Kumar Narayanan wrote <br>
<blockquote type="cite">Hello Prasanta <br>
<br>
Thanks for the explanation. <br>
Being new to the Printing subsystem, it helped get the
context of the problem. <br>
<br>
As I understand, the problem is due to
getDefaultPrinterSetings() returning negative values for
defYRes and quality. <br>
And the fix evades such negative values with a hardcoded low
300 DPI resolution. <br>
<br>
I 'm suspecting the bug to be in the code that returns
values for default printer settings <br>
. Since these values are device specific, I believe, the
code might use few platform APIs to query the resolutions on
that printer device & return the same. <br>
. It's here that default resolutions are not retrieved
properly. <br>
. So my view is to trace this location & fix the
population of default printer settings than a hardcoded DPI
resolution. <br>
. When a problem has surfaced on one printer, there is
possibility for the same to occur on many devices as well. <br>
. Besides printers may not be supporting low 300 DPI
resolution going forward. <br>
<br>
I may be wrong in my understanding. You could wait for
other's review & follow up. <br>
<br>
Thank you <br>
Have a good day <br>
<br>
Prahalad N. <br>
<br>
-----Original Message----- <br>
From: Prasanta Sadhukhan <br>
Sent: Thursday, August 31, 2017 3:39 PM <br>
To: Philip Race; 2d-dev <br>
Subject: [OpenJDK 2D-Dev] [10] RFR
JDK-8186987:NullPointerException in RasterPrinterJob without
PrinterResolution <br>
<br>
Hi All, <br>
<br>
Please review a fix for an issue where it a NPE is seen when
an attempt is made to print to Brother HL-2240D series
printer. <br>
<br>
It seems when RasterPrinterJob#setAttributes() is called
with no PrinterResolution attribute set, it first checks if
PrinterResolution category is supported. <br>
If it is supported, then it sees if the supplied resolution
value is supported. Now, since no PrinterResolution
attribute is set, so <br>
isSupportedValue() returns false [as "printer resolution
attribute" <br>
object is null] <br>
It then goes to get the default resolution attribute via <br>
getDefaultAttributeValue() which calls
getDefaultPrinterSettings() and use yRes,Quality from this
printer to construct a "PrinterResolution" <br>
object. <br>
<br>
Now, it seems in Brother HL-2240D series printer, it
supports 3 resolution [300, 600, HQ 1200] but for all these
3 resolutions, <br>
getDefaultPrinterSettings() returns -50 for yRes and
Quality. <br>
So, as per this code <br>
<a class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk10/client/jdk/file/dbb5b171a16b/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1189"
moz-do-not-send="true">http://hg.openjdk.java.net/jdk10/client/jdk/file/dbb5b171a16b/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1189</a>
<br>
res < 0 and no PrinterResolution object is instantiated
so when RPJ accesses
printerResAttr.getCrossFeedResolution(ResolutionSyntax.DPI);
it causes NPE. <br>
<br>
Proposed fix is to create a default lowly 300 dpi
PrinterResolution if, for some reason, yRes and Quality from
printer comes out -ve. <br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/8186987/webrev.00/"
moz-do-not-send="true">http://cr.openjdk.java.net/~psadhukhan/8186987/webrev.00/</a>
<br>
<br>
Regards <br>
Prasanta <br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic