Skip to content
This repository has been archived by the owner on May 26, 2018. It is now read-only.

Delegate checks to the existing SecurityManager. #507

Closed

Conversation

GUIpsp
Copy link

@GUIpsp GUIpsp commented Sep 2, 2014

In case one already exists.

Fixes #472

@GUIpsp
Copy link
Author

GUIpsp commented Sep 2, 2014

@Kubuxu it is only allowed to change once, the fml manager blocks further changes, thus it is still secure

@Kubuxu
Copy link

Kubuxu commented Sep 3, 2014

Allowing VM setting SecurityManager is IMHO not right practice. I suggest rebuilding forge for people that need SM - with just the trow commented out.
It is for safety and ease. In theory you can use build in but then be ready to read every commit in new forge version.
Control flow can sometimes go in weird direction.

@GUIpsp
Copy link
Author

GUIpsp commented Sep 3, 2014

@Kubuxu allowing setsecuritymanager is totally safe since fml blocks it.

@Kubuxu
Copy link

Kubuxu commented Sep 3, 2014

But future changes in fml might make something load before this and so on. So as I said, if you want to use be ready to audit every new commit in release.

@GUIpsp
Copy link
Author

GUIpsp commented Sep 3, 2014

@Kubuxu nothing will ever load before this. this is not the correct mechanism to sandbox servers anyways.

@GUIpsp
Copy link
Author

GUIpsp commented Sep 3, 2014

@Kubuxu also, reviewing every single fml commit is not that hard.

@Kubuxu
Copy link

Kubuxu commented Sep 4, 2014

But it is more work every time than setting Jenkins with patch file.

@LexManos
Copy link
Member

LexManos commented Sep 4, 2014

As discussed before, this is not a proper solution and causes immense performance decreases. People have yet t provide any form of valid reason to have this ability.

@LexManos LexManos closed this Sep 4, 2014
@GUIpsp
Copy link
Author

GUIpsp commented Sep 4, 2014

@LexManos This should only provide a performance decrease if the delegate exists no?

Yeah, and I agree it's a solution to a non-existing problem.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please Do Not use setSecurityManager Or createSecurityManager.
3 participants