6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified #2377

prsadhuk · 2021-02-03T12:11:40Z

Method createStrokedShape(Shape) for java.awt.BasicStroke class throws NullPointerException when passed a null object reference for a input parameter but it's not specified in the spec.
Updated spec to illustrate this.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified

Reviewers

Sergey Bylokhov (@mrserb - Reviewer)
Phil Race (@prrace - Reviewer) ⚠️ Review applies to d65b8e0
Alexey Ivanov (@aivanov-jdk - Reviewer) ⚠️ Review applies to d65b8e0

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2377/head:pull/2377
$ git checkout pull/2377

bridgekeeper · 2021-02-03T12:11:56Z

👋 Welcome back psadhukhan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-02-03T12:13:14Z

@prsadhuk The following labels will be automatically applied to this pull request:

2d
awt

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2021-02-03T12:15:55Z

Webrevs

prsadhuk · 2021-02-03T12:28:27Z

/label remove awt

openjdk · 2021-02-03T12:28:39Z

@prsadhuk
The awt label was successfully removed.

prsadhuk · 2021-02-03T12:31:01Z

/csr JDK-8261033

openjdk · 2021-02-03T12:31:14Z

@prsadhuk usage: /csr [needed|unneeded], requires that the issue the pull request refers to links to an approved CSR request.

prsadhuk · 2021-02-03T12:32:22Z

/csr

openjdk · 2021-02-03T12:32:56Z

@prsadhuk this pull request will not be integrated until the CSR request JDK-8261033 for issue JDK-6211257 has been approved.

mrserb · 2021-02-03T23:34:01Z

src/java.desktop/share/classes/java/awt/BasicStroke.java

@@ -294,6 +294,7 @@ public BasicStroke() {
     * stroked outline of a specified {@code Shape}.
     * @param s the {@code Shape} boundary be stroked
     * @return the {@code Shape} of the stroked outline.
+     * @exception NullPointerException if {@code Shape} is {@code null}


We use "@throws" in the new code.

any feedback on this?

aivanov-jdk · 2021-02-05T16:14:22Z

test/jdk/java/awt/BasicStroke/TestNullShape.java

+            System.out.println("result (npe): true");
+            return;
+        }
+        throw new RuntimeException("NPE is expected");


I would suggest moving this into the try-block as it would make the intention clearer: if NPE is not thrown, it's a failure.

Also catch should be on the same line as the closing brace of try.

mrserb · 2021-02-06T00:03:22Z

src/java.desktop/share/classes/java/awt/BasicStroke.java

@@ -294,6 +294,7 @@ public BasicStroke() {
     * stroked outline of a specified {@code Shape}.
     * @param s the {@code Shape} boundary be stroked
     * @return the {@code Shape} of the stroked outline.
+     * @throws NullPointerException if {@code Shape} is {@code null}


I think that you mean the parameter {@code s}, not a type Shape.

mrserb · 2021-02-06T00:05:06Z

test/jdk/java/awt/BasicStroke/TestNullShape.java

+            Shape s = bs.createStrokedShape(null);
+            System.out.println("result: false");
+        }
+        catch(NullPointerException ne) {


You can add a space after catch and move it to one line above.

This reverts commit 3fff74d.

aivanov-jdk · 2021-02-08T10:52:56Z

test/jdk/java/awt/BasicStroke/TestNullShape.java

-        }
-        catch(NullPointerException ne) {
+            throw new RuntimeException("NPE is expected");
+        } catch (NullPointerException ne) {
            System.out.println("result (npe): true");
            return;


Return is from catch is redundant now.

Suggested change

return;

openjdk · 2021-02-08T22:24:09Z

@prsadhuk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified

Reviewed-by: serb, prr, aivanov

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 116 new commits pushed to the master branch:

7451962: 8129776: The optimized Stream returned from Files.lines should unmap the mapped byte buffer (if created) when closed
ad525bc: 8261281: Linking jdk.jpackage fails for linux aarch32 builds after 8254702
2fd8ed0: 8240632: Note differences between IEEE 754-2019 math lib special cases and java.lang.Math
ace8f94: 8195744: Avoid calling ClassLoader.checkPackageAccess if security manager is not installed
ab65d53: 8261261: The version extra fields needs to be overridable in jib-profiles.js
20d7713: 8261334: NMT: tuning statistic shows incorrect hash distribution
92c6e6d: 8261254: Initialize charset mapping data lazily
351d788: 8259074: regex benchmarks and tests
d6d5d9b: 8261231: Windows IME was disabled after DnD operation
29a428f: 8261229: MethodData is not correctly initialized with TieredStopAtLevel=3
... and 106 more: https://git.openjdk.java.net/jdk/compare/181d63ffcee7a2f03ed9b8278b5ffface71a494f...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

prsadhuk · 2021-02-10T07:22:06Z

/integrate

openjdk · 2021-02-10T07:23:06Z

@prsadhuk Since your change was applied there have been 136 commits pushed to the master branch:

4619f37: 8261472: BasicConstraintsExtension::toString shows "PathLen:2147483647" if there is no pathLenConstraint
699a3cd: 8223188: Removed unnecessary #ifdef __cplusplus from .cpp sources
05c6009: 8259656: fixpath.sh changes broke _NT_SYMBOL_PATH in RunTests.gmk
ef7ee3f: 8225081: Remove Telia Company CA certificate expiring in April 2021
7c565f8: 8261209: isStandalone property: remove dependency on pretty-print
01d9280: 8261299: Use-after-free on failure path in LinuxPackage.c, getJvmLauncherLibPath
a00b130: 8261356: Clean up enum G1Mark
becee64: 8261279: sun/util/resources/cldr/TimeZoneNamesTest.java timed out
f395ee0: 8261306: ServiceLoader documentation has malformed Unicode escape
8f4c15f: 8198540: Dynalink leaks memory when generating type converters
... and 126 more: https://git.openjdk.java.net/jdk/compare/181d63ffcee7a2f03ed9b8278b5ffface71a494f...master

Your commit was automatically rebased without conflicts.

Pushed as commit 3af334a.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

prsadhuk added 2 commits February 3, 2021 12:07

6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified

c439c2a

6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified

899868c

openjdk bot added the rfr Pull request is ready for review label Feb 3, 2021

openjdk bot added 2d client-libs-dev@openjdk.org awt client-libs-dev@openjdk.org labels Feb 3, 2021

openjdk bot removed the awt client-libs-dev@openjdk.org label Feb 3, 2021

openjdk bot added the csr Pull request needs approved CSR before integration label Feb 3, 2021

mrserb reviewed Feb 3, 2021

View reviewed changes

Doc change to @throws

f7d6e8f

aivanov-jdk reviewed Feb 5, 2021

View reviewed changes

mrserb suggested changes Feb 6, 2021

View reviewed changes

prsadhuk added 3 commits February 7, 2021 09:11

Address review comments

3fff74d

Revert "Address review comments"

c4290c0

This reverts commit 3fff74d.

Address review comments

d65b8e0

prrace approved these changes Feb 7, 2021

View reviewed changes

aivanov-jdk approved these changes Feb 8, 2021

View reviewed changes

Remove redundant return statement

bdc97f6

mrserb approved these changes Feb 8, 2021

View reviewed changes

openjdk bot removed the csr Pull request needs approved CSR before integration label Feb 8, 2021

openjdk bot added the ready Pull request is ready to be integrated label Feb 8, 2021

openjdk bot closed this Feb 10, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 10, 2021

prsadhuk deleted the JDK-6211257 branch February 10, 2021 07:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified #2377

6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified #2377

prsadhuk commented Feb 3, 2021 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 3, 2021

openjdk bot commented Feb 3, 2021

mlbridge bot commented Feb 3, 2021 •

edited

Loading

prsadhuk commented Feb 3, 2021

openjdk bot commented Feb 3, 2021

prsadhuk commented Feb 3, 2021

openjdk bot commented Feb 3, 2021

prsadhuk commented Feb 3, 2021

openjdk bot commented Feb 3, 2021

mrserb Feb 3, 2021

prsadhuk Feb 5, 2021

aivanov-jdk Feb 5, 2021

mrserb Feb 6, 2021

mrserb Feb 6, 2021

aivanov-jdk Feb 8, 2021

openjdk bot commented Feb 8, 2021

prsadhuk commented Feb 10, 2021

openjdk bot commented Feb 10, 2021

6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified #2377

6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified #2377

Conversation

prsadhuk commented Feb 3, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Feb 3, 2021

openjdk bot commented Feb 3, 2021

mlbridge bot commented Feb 3, 2021 • edited Loading

Webrevs

prsadhuk commented Feb 3, 2021

openjdk bot commented Feb 3, 2021

prsadhuk commented Feb 3, 2021

openjdk bot commented Feb 3, 2021

prsadhuk commented Feb 3, 2021

openjdk bot commented Feb 3, 2021

mrserb Feb 3, 2021

Choose a reason for hiding this comment

prsadhuk Feb 5, 2021

Choose a reason for hiding this comment

aivanov-jdk Feb 5, 2021

Choose a reason for hiding this comment

mrserb Feb 6, 2021

Choose a reason for hiding this comment

mrserb Feb 6, 2021

Choose a reason for hiding this comment

aivanov-jdk Feb 8, 2021

Choose a reason for hiding this comment

openjdk bot commented Feb 8, 2021

prsadhuk commented Feb 10, 2021

openjdk bot commented Feb 10, 2021

prsadhuk commented Feb 3, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Feb 3, 2021 •

edited

Loading