[BUG] FML : FMLSecurityManager

[RedRelay]

Il semblerait qu’il y ait une erreur dans la classe net.minecraftforge.fml.relauncher.FMLSecurityManager, je vous laisse chercher laquelle
Même les plus débutant peuvent trouver !

PS : ou alors c’est moi qui ne comprend pas cette feature ^^

 
package net.minecraftforge.fml.relauncher;
 
import java.security.Permission;
 
/**
 * A custom security manager stopping certain events from happening
 * unexpectedly.
 *
 * @author cpw
 *
 */
public class FMLSecurityManager extends SecurityManager {
    @Override
    public void checkPermission(Permission perm)
    {
        String permName = perm.getName() != null ? perm.getName() : "missing";
        if (permName.startsWith("exitVM"))
        {
            Class[] classContexts = getClassContext();
            String callingClass = classContexts.length > 3 ? classContexts[4].getName() : "none";
            String callingParent = classContexts.length > 4 ? classContexts[5].getName() : "none";
            // FML is allowed to call system exit and the Minecraft applet (from the quit button)
            if (!(callingClass.startsWith("net.minecraftforge.fml.")
                    || "net.minecraft.server.dedicated.ServerHangWatchdog$1".equals(callingClass)
                    || "net.minecraft.server.dedicated.ServerHangWatchdog".equals(callingClass)
                    || ( "net.minecraft.client.Minecraft".equals(callingClass) && "net.minecraft.client.Minecraft".equals(callingParent))
                    || ("net.minecraft.server.dedicated.DedicatedServer".equals(callingClass) && "net.minecraft.server.MinecraftServer".equals(callingParent)))
                    )
            {
                throw new ExitTrappedException();
            }
        }
        else if ("setSecurityManager".equals(permName))
        {
            throw new SecurityException("Cannot replace the FML security manager");
        }
        return;
    }
 
    @Override
    public void checkPermission(Permission perm, Object context)
    {
        this.checkPermission(perm);
    }
 
    public static class ExitTrappedException extends SecurityException {
        private static final long serialVersionUID = 1L;
    }
}
 

[Mokona78]

Une histoire d’indices peut-être ?

[RedRelay]

@‘Mokona78’:

Une histoire d’indices peut-être ?

Oui ^^ Et ça m’embête parce que ça fait crash mon appli
Je suis entrain de faire un TestRunner custom pour pouvoir faire des tests unitaires JUnit ^^

[Axaurus]

@‘Mokona78’:

Une histoire d’indices peut-être ?

Je ne comprend pas …

Sinon, j’aurais dit un return inutile car la méthode est void, mais visiblement ce n’est pas ça

[robin4002]

String callingClass = classContexts.length > 3 ? classContexts[4].getName() : “none”;
String callingParent = classContexts.length > 4 ? classContexts[5].getName() : “none”;

[SCAREX]

Donc techniquement pour le résoudre le seul moyen c’est de passer par une autre méthode afin de ne pas avoir d’IndexOutOfBounds exception, si j’ai bien compris ?

[robin4002]

ou de lui envoyer 6 arguments.

[Mokona78]

Et concrètement, elle sert à quoi cette fonction ?

J’ai regardé un peu, elle est appelée dans le cas qui pourrait lancer une exception lorsque l’on quitte le jeu.
Ca ne crash pas car à ce moment, la callstack est plus profonde que 6.

Mais…

1/ Pourquoi est-elle appelée ? / À quoi ça sert de vérifier dans quel contexte d’exécution on est ?
2/ Pourquoi est-ce qu’elle gêne dans les tests unitaires ?

[SCAREX]

Le FMLSecurityManager est là pour empêcher que les mods quittent le jeu ou bien changent le security manager.

Elle doit gêner pour les tests unitaires car ils doivent fermer l’application de force je pense

[Mokona78]

Mais qu’est-ce qui empêche les mods d’aller patcher le bytecode du Security Manager ?

[RedRelay]

1/ Elle permet de ne pas pouvoir changer le SecurityException de façon standard et de levé une exception dès qu’une classe autre que FML ou Minecraft appelle la fonction System.exit().
Bien que l’exception levé fasse crasher l’appli, elle permet de générer un rapport d’erreur. Ça éviter aux joueurs de signaler un bug sans crash log lorsqu’un moddeur cochon utilise System.exit() (et il y en a sur ce forum).

2/ Mon Test Runner JUnit custom, est lancé sur Eclipse via “org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main()”. C’est cette classe qui appel System.exit() à la fin de tout les tests.
Donc les test sont exécuté, mais j’ai un crash dans la console et j’aime pas, ça fait sale ^^

Rien n’empêche les mods d’aller patcher le bytecode du SecurityManager, c’est d’aller ce que j’ai fais ^^
(Mais c’est d’un bordel monstre avec les différents ClassLoader, ça fais 4 jours que je suis dessus, il y a plein de petite subtilité a savoir. C’est pas forcément compliqué, mais ce sont des choses qu’ils faut savoir ^^)

Je n’étais pas obliger de report le bug, mais tant qu’a faire ^^
Je doute que beaucoup de personnes ait l’occasion de dealer avec cette classe donc j’ai fais une pull request ^^
(Je vais passer contributeur MinecraftForge, la classe )

[SCAREX]

Normalement le Security manager de fml ne peut pas être patché puisqu’il est chargé avant les plugins fml

[RedRelay]

Je n’utilise pas de coremod pour patcher le SecurityManager.
En gros, dans le constructeur de mon TestRunner, voila ce que j’ai :

 
public MinecraftServerJUnitRunner(Class klass) throws InitializationError {
super(klass);
ASMTransformer.injectCode();
try {
Class.forName("GradleStartServer", true, Thread.currentThread().getContextClassLoader()).getDeclaredMethod("main", String[].class).invoke(null, new Object[] { params });
} catch (Throwable e) {
throw new InitializationError("Unable to launch Minecraft Server : "+e.getMessage());
}
 
System.out.println("Waiting for Minecraft Server start …");
/* ... */
 

Donc je fais les modifications avant de lancer le jeu.
Les classes sont bien modifié, mais je n’arrive pas a les lancer, à chaque fois je retombe sur les classes originale …
Donc pour l’instant je cherche… pourtant j’ai créer un nouveau ClassLoader pour avoir accès a la fonction defineClass(classname, byte[], offset, length)
Et je met ce classLoader dans Thread.currentThread().setContextClassLoader() …

Je modifie la class GradleStartCommon pour remplacer le Class.forName(String name) par Class.forName(String name, boolean init, ClassLoader cl)
Je modifie la class Launch pour que mettre mon CustomClassLoader en parent du nouveau ClassLoader et lui ajouter une classLoaderExcusion
Je modifie la class FMLSecurityManager en supprimant toutes les instructions dans checkPermission()

Ça a marché une fois il me semble, alors j’ai factorisé mon code pour que se soit plus propre et la ça ne fonctionne plus …
Je m’en tire les cheveux ><

[Mokona78]

@‘Blackout’:

Je n’étais pas obliger de report le bug, mais tant qu’a faire ^^
Je doute que beaucoup de personnes ait l’occasion de dealer avec cette classe donc j’ai fais une pull request ^^

C’est la bonne attitude !

Merci pour les précisions.

[RedRelay]

@‘Mokona78’:

@‘Blackout’:

Je n’étais pas obliger de report le bug, mais tant qu’a faire ^^
Je doute que beaucoup de personnes ait l’occasion de dealer avec cette classe donc j’ai fais une pull request ^^

C’est la bonne attitude !

Merci pour les précisions.

Parfois je me le demande, ma pull request a été refuser avec le message :
@‘cpw’:

[font=-apple-system, BlinkMacSystemFont,No error log? I’ve never seen this happen. Hypothetical fixes are not accepted.]

J’hallucine …

[SCAREX]

Ce que tu peux faire à la place c’est de re-créer le FMLSecurityManager et de charger le bytecode depuis un autre fichier

[Mokona78]

@‘Blackout’:

@‘Mokona78’:

@‘Blackout’:

Je n’étais pas obliger de report le bug, mais tant qu’a faire ^^
Je doute que beaucoup de personnes ait l’occasion de dealer avec cette classe donc j’ai fais une pull request ^^

C’est la bonne attitude !

Merci pour les précisions.

Parfois je me le demande, ma pull request a été refuser avec le message :
@‘cpw’:

[font=-apple-system, BlinkMacSystemFont,No error log? I’ve never seen this happen. Hypothetical fixes are not accepted.]

J’hallucine …

C’est celui qui a refusé qui n’a pas la bonne attitude…

[Superloup10]

cpw est quand même plus sympa avec les pull request que Lex, en revanche, les tests unitaires ne sont pas considérés comme une utilisation normale de FML et de Forge d’où le rejet.

[Mokona78]

Mais là, il y a une erreur non ? Elle n’est pas bien complexe à reproduire j’imagine. Il suffit d’appeler la fonction dans un contexte avec une pile moins profonde que 5 niveaux.

À moins que ce soit impossible ?

[Superloup10]

Si c’est une erreur très grave, il y a de forte chance que d’autres moddeurs aient eu le même souci, personnellement, j’ai beau avoir relue une bonne dizaine de fois le code qu’à donné Blackout, je ne vois toujours pas où est le problème.