-
Notifications
You must be signed in to change notification settings - Fork 182
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
fixes #1242 #1246
base: master
Are you sure you want to change the base?
fixes #1242 #1246
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1246 +/- ##
==========================================
- Coverage 87.19% 87.17% -0.02%
==========================================
Files 113 113
Lines 10296 10296
Branches 4051 4051
==========================================
- Hits 8978 8976 -2
- Misses 726 727 +1
- Partials 592 593 +1
|
@Thrameos I need a nap cause I can't think but I did confirm this was the problem. There could be "security" arguments about this but I think they're all 💩 . |
Stupid me forgot about std::wstring, that'll make this easier. Should be good now I think. |
I am having a hard time fathoming how we could ever be in a situation where the JVM loaded itself using another versions dll. This seems extraordinarily broken. Is there any chance of this on the linux version? |
Is anything new calling just "java" vs using JAVA_HOME? |
I watched it happen in the debugger and can show you the logs if necessary. I have no idea if this can happen on linux or Mac. Windows has stupid dll path search rules. The issue is this specificaly, it just now results in a different error message. logsThe log below is from running the following. Note that
|
No. |
The easiest way to check if it occurs on Linux would be to get the zip file of the jdk, extract it somewhere and then without adding it to PATH, specify the absolute path to the jvm.so in |
I don't know what to say other then, no this was not a security issue and no this doesn't introduce one.
fixes #1242