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

[GH-16351] Do not call System.exit from water.tools [nocheck] #16366

Merged
merged 7 commits into from
Oct 23, 2024

Conversation

krasinski
Copy link
Member

fix #16351

@krasinski krasinski self-assigned this Aug 14, 2024
@krasinski krasinski linked an issue Aug 14, 2024 that may be closed by this pull request
@wendycwong
Copy link
Collaborator

@krasinski

Your fix has stopped h2o-3 cluster from crashing. I ran the following:

curl http://localhost:54321/99/Rapids --data-urlencode 'ast=(run_tool "MojoConvertTool" ["", "", "", ""])'

and got the following result:

image

From talking to @mmalohlava , he does not want to see the whole stack, just the error message which in this case is

"Caused by:java.lang.IllegalArgumentException: Specified MOJO file (/Users/wendycwong/h2o-3) doesn't exist!",

I took a look and think this is the file that passes everything over:

RapidsHandler.java around line 38 where the whole stack errors are passed as well.

image

mainInternal(args);
}
catch (IllegalArgumentException e) {
System.err.println(e.getMessage());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please make sure this only print out the actual error message and not the whole stack.

@wendycwong
Copy link
Collaborator

I ran your code again today (sept 4, 2024) but still got the following:

image

@wendycwong
Copy link
Collaborator

wendycwong commented Oct 15, 2024

@krasinski : Just run the command again and got the following error in one line, I save it to a file and added newline to it so I can read it and here it is:

image

It does not look like the error message is passed down to the user.

@wendycwong
Copy link
Collaborator

I get to see the error message clearly at the end. Even though there are other stuff with it, I think we are done making changes here.

@wendycwong wendycwong changed the title [GH-16351] Do not call System.exit from water.tools [GH-16351] Do not call System.exit from water.tools [nocheck] Oct 22, 2024
@wendycwong wendycwong merged commit d0899f8 into rel-3.46.0 Oct 23, 2024
72 checks passed
@wendycwong wendycwong deleted the gh-16351-main-internal branch October 23, 2024 15:36
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.

Fix CVE-2024-5979 AstRunTool
2 participants