Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSUE #3963] Method invocation 'getBytes' may produce 'NullPointerException'. #3971

Merged
merged 4 commits into from
May 31, 2023

Conversation

ZahaanMahajan
Copy link
Contributor

@ZahaanMahajan ZahaanMahajan commented May 19, 2023

Fixes #3963

Method invocation 'getBytes' may produce 'NullPointerException'.

In the modified code, Objects.requireNonNull() is applied to the return value of JsonUtils.toJSONString(result). If the result is null, a NullPointerException will be thrown immediately, preventing the subsequent invocation of getBytes().

By using Objects.requireNonNull(), it explicitly check for nullness and handle it in a more controlled manner, ensuring that it don't encounter unexpected null values during the execution of the code.

mxsm
mxsm previously approved these changes May 19, 2023
Copy link
Member

@mxsm mxsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM~

Alonexc
Alonexc previously approved these changes May 19, 2023
Copy link
Contributor

@Alonexc Alonexc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -60,7 +61,7 @@ public void handle(HttpExchange httpExchange) throws IOException {

try (OutputStream out = httpExchange.getResponseBody()) {
WebHookConfig result = operation.queryWebHookConfigById(webHookConfig); // operating result
out.write(JsonUtils.toJSONString(result).getBytes(Constants.DEFAULT_CHARSET));
out.write(Objects.requireNonNull(JsonUtils.toJSONString(result).getBytes(Constants.DEFAULT_CHARSET)));
Copy link
Contributor

@Alonexc Alonexc May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be done:
out.write(Objects.requireNonNull(JsonUtils.toJSONString(result)).getBytes(Constants.DEFAULT_CHARSET));

@@ -73,7 +73,7 @@ public void handle(HttpExchange httpExchange) throws IOException {

try (OutputStream out = httpExchange.getResponseBody()) {
List<WebHookConfig> result = operation.queryWebHookConfigByManufacturer(webHookConfig, pageNum, pageSize); // operating result
out.write(JsonUtils.toJSONString(result).getBytes(Constants.DEFAULT_CHARSET));
out.write(Objects.requireNonNull(JsonUtils.toJSONString(result).getBytes(Constants.DEFAULT_CHARSET)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok got it

@ZahaanMahajan ZahaanMahajan dismissed stale reviews from Alonexc and mxsm via 7c8e3ac May 19, 2023 08:42
@xwm1992 xwm1992 changed the title fixes [Enhancement] #3963 [ISSUE #3963] Method invocation 'getBytes' may produce 'NullPointerException'. May 22, 2023
@xwm1992
Copy link
Contributor

xwm1992 commented May 29, 2023

@ZahaanMahajan you can merge the latest master branch code to fix the ci check error.

@ZahaanMahajan
Copy link
Contributor Author

ZahaanMahajan commented May 29, 2023

I didn't get you. How to do that?

@xwm1992
Copy link
Contributor

xwm1992 commented May 29, 2023

I didn't get you. How to do that?

pull apache:master to your local branch and push it to your remote master branch.

@codecov
Copy link

codecov bot commented May 29, 2023

Codecov Report

Merging #3971 (0a5a912) into master (4db7667) will not change coverage.
The diff coverage is 0.00%.

❗ Current head 0a5a912 differs from pull request most recent head 840ccfb. Consider uploading reports for the commit 840ccfb to get more accurate results

@@            Coverage Diff            @@
##             master    #3971   +/-   ##
=========================================
  Coverage     14.25%   14.25%           
  Complexity     1320     1320           
=========================================
  Files           579      579           
  Lines         28943    28943           
  Branches       2794     2794           
=========================================
  Hits           4125     4125           
  Misses        24426    24426           
  Partials        392      392           
Impacted Files Coverage Δ
...e/admin/handler/QueryWebHookConfigByIdHandler.java 26.66% <0.00%> (ø)
...ndler/QueryWebHookConfigByManufacturerHandler.java 28.57% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ZahaanMahajan ZahaanMahajan requested review from Alonexc and mxsm May 29, 2023 16:32
@xwm1992 xwm1992 merged commit c5f3aba into apache:master May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Method invocation 'getBytes' may produce 'NullPointerException'.
5 participants