code update

[emecdelam]

Allow to compile and execute code in rust,java,python

Friendly interface

Using modals to submit code

Alternatively use file to submit code

Returns file less than 8mb for long outputs

Setup

Languages

To allow the extension to work properly, you need to have rust setup on the machine, as well as python. Once python installed you will need to use

pip install numpy

pip install scipy

Environment variables

you will need JAVA_HOME to be a path environment variable that redirect to jdk, something like C:\Program Files\Java\jdk-20
as well as python and rustc

Security

Since we are running code without dockers (choice made to avoid installing the lib since its not my repo/project)
There are security concerns.

Rust

regarding rust, the use and std are banned keywords this remove the possibility to import amything

Java

for java "java.lang.reflect","java.io","java.util.zip","java.net","java.nio.file","java.security","java.awt","javax.swing","javax.script","java.util.logging","java.lang.ProcessBuilder","java.lang.Runtime","com","net","org"
are banned however I doubt it patches everything ideally it should run in a docker

Python

for python, it removes import,open,exec and eval

Overall

usefull to debug and explain code as it renders the errors

but you might need to consider docker for perfect security

[Hokkaydo]

Thank you for your contribution.
This seems a nice feature.
However, when I look at your commits, I see a lot of unnecessary files (compilation and debug outputs). Please consider cleaning it before proceeding further.
Moreover, I don't understand why you used Rust here. What you did here can be likely achieved in Java.
Regarding your concerns with dockerization, the application is run in a docker container. Although it's one of my first uses of "Docker", it looks good for security. If you have any improvements to suggest, don't hesitate.

[emecdelam]

I had to use a rust library to access python since the jython library is old and support python 2.7
not python 3, while pyo3 is up to date.
Regarding security, the most dangerous exploit would be accessing the token since it would allow to create another bot using the token and ban every user/delete every channel

public class TokenGrabber{
    public static void main(String[] args) {
        System.out.println(System.getenv("DISCORD_BOT_TOKEN"));
    }
}

to avoid this sort of trouble I recommand putting directly the token in the Main.java
since imports starting with com. are disabled while System.getenv is not.

The compiled files from the library have been deleted, to recreate them just use

cargo build

after navigating to EPLBot\src\main\java\com\github\hokkaydo\eplbot\dllrun\mylib\src\lib.rs

[Hokkaydo]

Hello,
Sorry for not getting back to you sooner, I didn't find time earlier. I looked at your code and have a few changes I'd like you to implement.
But first, I couldn't test your code :

• You plan to generate a .dll file, and you pass an absolute path to "mylib.dll". However, on Linux (the OS the bot runs on), cargo generates a "libmylib.so" file and your path is wrong. I don't know if there is a way to give a particular name to the lib in the cargo config file
• The second problem lies in the absolute path. The code is not copied as is to the container and executed. The Java code is packaged in a .jar file but your Rust code and library are not. A fix could be to modify the Dockerfile to COPY the previously generated library to a particular directory in the container (perhaps /home/eplbot/lib) which would then be referenced in the Java call to the library.

Maybe I didn't understand how cargo works or I'm wrong. I apologize if this is the case.

About the quick code changes :

• I think it might be easier to read the code (and use it as a user) if the /python, /java, /rust commands were assembled into a single command /compile [java|python|rust] command.
• Rename the packages following the Java convention: snake_case
• Remove unnecessary imports (there are many)
• Rename your constants (private static final): SCREAMING_SNAKE_CASE
• Sometimes you assign a value and return it immediately, just return the call (instead of String ret = call(...); return ret;, do return call(...);
  There are other code style changes I want to implement, but I'll do them myself because they're hard to explain.

Also, if you have a way around the environment variables issue, I'm open to it. I won't put the bot token directly into Main.java because this code ends up on the public Github. I'm thinking of running user code in a Docker container itself.

[emecdelam]

Quick view : Fixing and optimisation

MINUS

• rust library to interact with python
• unused imports
• unnecessary variables

PLUS

• snake_case for packages
• SNAKE_CASE for private static final variables
• compile command
• pythonRunner class
• changes to modal

In depth

Rust library

The rust library used to interact with python as been replaced with the pythonRunner class, it should remove compatibilty errors between os.

Imports, variables, snake_case

The unused imports should have been removed, unnecessary variables too, snake_case should be of use for most important variables, I haven t checked for every variables tho ( I'm speaking of the one used in a function)

Modal and compile command

Added a /compile [Optional]{python,rust,java} [Optional]$file$
However it appears that having a list of options overrides the 'required' boolean meaning you can't just use /compile to open a modal
The modal has also been changed since you can t access the CommandContext.options() from an @NotNull ModalInteractionEvent event meaning you can t access the /compile [first_value] to see wich language should be compiled the result is

It might be possible but I haven't seen how for now.

Security

For the bot, the list of banned words when using a language is restrictive, if the bot is to be used in a docker, and that no important data is stored in the docker i don t see any trouble allowing every imports, this would allow users to delete some file from the both and access the data and code stored in but for a community trustfull it shouldn t happen. The safety checks make a balance between security and restriction. Up to you to decide wether they should be removed or not. Their main downside it that they dont allow for such code to run

use std::cell::Cell;
use std::cmp::Ordering;
use std::collections::{BinaryHeap, HashMap};
use std::fmt;
use std::hash::{Hash, Hasher};

fn main() {
    let s = Vertex::new("s");
    let t = Vertex::new("t");
    let x = Vertex::new("x");
    let y = Vertex::new("y");
    let z = Vertex::new("z");

    let mut adjacency_list = HashMap::new();
    adjacency_list.insert(&s, vec![(&t, 10), (&y, 5)]);
    adjacency_list.insert(&t, vec![(&y, 2), (&x, 1)]);
    adjacency_list.insert(&x, vec![(&z, 4)]);
    adjacency_list.insert(&y, vec![(&t, 3), (&x, 9), (&z, 2)]);
    adjacency_list.insert(&z, vec![(&s, 7), (&x, 6)]);

    dijkstra(&s, &adjacency_list);

    adjacency_list.keys().for_each(|v| println!("{:?}", v));
}

fn dijkstra<'a, 's: 'a>(
    start: &'a Vertex<'s>,
    adjacency_list: &'a HashMap<&'a Vertex<'s>, Vec<(&'a Vertex<'s>, usize)>>,
) {
    start.distance.set(0);

    let mut to_visit = BinaryHeap::new();
    adjacency_list.keys().for_each(|v| to_visit.push(*v));

    while let Some(v) = to_visit.pop() {
        if let Some(neighbors) = adjacency_list.get(v) {
            for (n, cost) in neighbors {
                let new_distance = v.distance.get() + cost;
                if new_distance < n.distance.get() {
                    n.distance.set(new_distance);
                }
            }
            let mut new_heap = BinaryHeap::new();
            to_visit.iter().for_each(|x| new_heap.push(*x));
            to_visit = new_heap;
        }
    }
}

struct Vertex<'a> {
    name: &'a str,
    distance: Cell<usize>,
}

impl<'a> Vertex<'a> {
    fn new(name: &'a str) -> Vertex<'a> {
        Vertex {
            name,
            distance: Cell::new(usize::max_value()),
        }
    }
}

impl<'a> Hash for Vertex<'a> {
    fn hash<H: Hasher>(&self, state: &mut H) {
        self.name.hash(state);
    }
}

impl<'a> Ord for Vertex<'a> {
    fn cmp(&self, other: &Vertex<'a>) -> Ordering {
        other.distance.get().cmp(&self.distance.get())
    }
}

impl<'a> PartialOrd for Vertex<'a> {
    fn partial_cmp(&self, other: &Vertex<'a>) -> Option<Ordering> {
        Some(self.cmp(other))
    }
}

impl<'a> PartialEq for Vertex<'a> {
    fn eq(&self, other: &Vertex<'a>) -> bool {
        self.name == other.name
    }
}

impl<'a> fmt::Debug for Vertex<'a> {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "name: {}, distance: {:?}", self.name, self.distance.get())
    }
}

Regarding the token you can always encrypt it and use a docker external program to decrypt it, it wouldn t impact the repository code (-> .gitignore) but would allow you to be more secure.

Don t hesitate to ask for changes instead of making them yourself, you can also point toward the line of code to change like this

Lastly

Now the code is shorter and should remove this kind of concern

spoiler

yes I made it alone

[emecdelam]

Update : time limit and fixes

Timelimit

I also added a time limit to run process actually of 60 secs, done to avoid running infinite loop for nothing. It can be changed in the strings.json file.

Fixes

Fixing error on python by replacing " in the inputs to ', when using " it creates the following problem : python -c "print("hello")" -> hello not defined
Also added error supports for non valid inputs ie: no main method in a rust file, no function to call, no class to call etc

[Hokkaydo]

Again, sorry for the late, not as much time as I hoped.
As said in the comments, I'm not a huge fan of reflection because of all the trouble it brings.
Aside from that, I added a couple of comments about translating messages in French and putting them in strings.json file.
There are also some quick variable names and code fixes.

However, I like this feature so keep going, it looks great!

Ps: When I started the review, you hadn't implemented a time limit yet and I was ready to suggest it but you did it so ... nicely done!

[Hokkaydo]

Would be nice to use the configuration system for this value. It allows moderators to update it on the fly without recompiling the code.

[emecdelam]

I implemented it but did n t dive deep into the module configuration code so don t know how to test it

[Hokkaydo]

Fine I'll do it when I find time

[emecdelam]

Code Update

• Added most of the requested changes

Non added-changes / waiting to be tested

#36 (comment)

 } catch (IOException | InterruptedException e) {
->
 } catch (IOException e) {

in PythonRunner.java due to

if (process.waitFor() == 0)

throwing InterruptedException

#36 (comment)
I dont like the IOException try catch on readfromfiles implementation especially the content = ""; part

#36 (comment)
The implementation should work but I m not sure how to test it and wich command to call in order to access the variables from a mod perspective

[Hokkaydo]

Okay sounds good
I will review the full code once more before merging.
For mod testing you have /config command to change the config for your server on the fly as well as /enable /disable to manage modules

[Hokkaydo]

Added a couple of things I want changes in.
Moreover, in the Java runner, I would like you to implement the possibility of sending Java code without the wrapping by passing an option (maybe "--wrapped" or what...). It would then put the code in a template class with a dummy public static void main and a couple of handy imports. It would allow the execution of such code :

int test = 4;
double sqrt = Math.sqrt(test);
System.out.println(test);

[emecdelam]

So working on implementing the --wrapped, I found it easier to just detect if the code needs to be wrapped

public static boolean requiresWrapper(String javaCode) {
        boolean hasClass = Pattern.compile("\\bpublic\\s+class\\s+[A-Z][a-zA-Z0-9]*").matcher(javaCode).find();
        boolean hasMainMethod = Pattern.compile("\\bpublic\\s+static\\s+void\\s+main\\s*\\(\\s*String\\[\\]\\s+[a-zA-Z0-9]*\\s*\\)").matcher(javaCode).find();
        boolean hasImports = Pattern.compile("\\bimport\\s+[a-zA-Z0-9.]+").matcher(javaCode).find();
        if (hasClass && hasMainMethod) {
            return false;
        } else if (hasImports) {
            return false;
        } else {
            return true;
        }
    }
    public static String addWrapper(String input){
        return """
                import java.util.*;
                import java.lang.Math;
                public class Wrapper{
                    public static void main(String[] args){""" + 
                        input +
                    """ 
                    }
                }""";
    }

using this.
Can I stick with this instead of the wrapped argument?

[Hokkaydo]

Yep it looks even better

[emecdelam]

Applying the requested changes

+ wrapper for java
+ file separator
+ cleaner messageLength function
- Big try catch block

Try-Catch

Regarding the try catch blocks for running processes, I tried to minimize the number functions they encapsulate however for rust, since the process of compiling and running may create allot of IOException to handle this the process may return 5 error named like this :

catch (IOException e) {
    e.printStackTrace();
    return "Server error code R01" + e.toString();
}

wrapper

    public static boolean requiresWrapper(String javaCode) {
        boolean hasClass = Pattern.compile("\\bpublic\\s+class\\s+[A-Z][a-zA-Z0-9]*").matcher(javaCode).find();
        boolean hasMainMethod = Pattern.compile("\\bpublic\\s+static\\s+void\\s+main\\s*\\(\\s*String\\[\\]\\s+[a-zA-Z0-9]*\\s*\\)").matcher(javaCode).find();
        boolean hasImports = Pattern.compile("\\bimport\\s+[a-zA-Z0-9.]+").matcher(javaCode).find();
        if (hasClass && hasMainMethod) {
            return false;
        } else if (hasImports) {
            return false;
        } else {
            return true;
        }
    }
    public static String addWrapper(String input){
        return """
                import java.util.*;
                import java.lang.Math;

                public class Wrapper {
                    public static void main(String[] args){""" + 
                        input +
                    """ 
                    }
                }""";
    }

The code used to detect the need for a wrapper doesn t support custom imports, its not planned to be added as its utility seems pointless knowing that every java.util are imported and writing/editing files isn t allowed

[Hokkaydo]

Left another couple of changes and some batches.
Additionally, if you have time (if not I'll do it), it would be nice to implement a C runner

[emecdelam]

I can add a C compiler but not for now as C is not memory safe and I fear troubles since it has a large amount of exploits, if it is added it will probably be when the dockers get added wich will probably be made after the exam session. For now I have implemented the requested changes

[Hokkaydo]

You're right. I'll implement it later with containers.
For now, please rebase your branch onto the dev branch and fix the few conflicts showing up

[Hokkaydo]

Last changes!
Once done, it should be immediately mergeable (if you rebase onto dev before of course)

[emecdelam]

Should be done

[Hokkaydo]

After some testing, here's what I found

• Language argument should be required and lowercased ("Java" produces an error while "java" does not)
• Remove the language entry from the modal
• As the docker image is initially a JDK image, there's no python interpreter nor rust compiler. Need a way to include it in the docker image (otherwise, python and rust runner can't execute)

For installing rust in the image, adding RUN curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain stable --default-host x86_64-unknown-linux-gnu --profile default -y after the FROM ... line should do the trick

[Hokkaydo]

Hello,
Any news ?

[emecdelam]

Not rn,

• I'm not sure what Language argument should be required and lowercased ("Java" produces an error while "java" does not)" means, in wich context? the modal or the appcommand
• Well, somehow it doesn't work, you can't have optional and non-optional arguments but I'm not done testing, I tryed by simply flipping the boolean in the context.arguments
• Solutions to this should be venv or python installation not sure wich is better, won't push until everything works.

[Hokkaydo]

Not rn,

* I'm not sure what `Language argument should be required and lowercased ("Java" produces an error while "java" does not)" means`, in wich context? the modal or the appcommand

Lowercased in appcommand and make it required. If so, you doesn't need to ask for language name in appmodal because the user already inputted it

* Well, somehow it doesn't work, you can't have optional and non-optional arguments but I'm not done testing, I tryed by simply flipping the boolean in the context.arguments

In that case, I think going for the modal only version is fine.

* Solutions to this should be venv or python installation not sure wich is better, won't push until everything works.

We'll need both venv and python installation in the docker script

[emecdelam]

Applied requested changes

• changed docker file to download python and rustc
• fixed some typo and merge conflict errors

Problem regarding wsl

Why do i need to have java installed to firstly build the project and then build the docker image ? To run it on WSL i have to install and setup java and its kinda fastidious, why don't you build the project inside of the docker container during the image build?

Testing

As of now it seems to work just fine for file and modal interactions. I plan to keep both options as copy pasting large file is not possible due to the 4000 characters limit on modal text inputs.

[emecdelam]

Code refactoring

Summary

• removed rust code integration
• changed from a global command to a module
• added docker containers for every code submission
• added c as a possible language
• fixed dual modal missing interaction error
• added docs to add another language in CONTRIBUTIONS.md
• added javadoc and docs to the code module
• make the command run dockers according to the language
• create a build_code_docker.sh to build all the dockers used for the code module
• set the timeout to 30 secs, enough for any reasonable calculation
• fix readme docker run --rm appearing twice
• adding a new parameter when building the docker : -v /var/run/docker.sock:/var/run/docker.sock
• execute codes asynchronously with a maximum of 10 processes
• has scipy and numpy in the python docker
• wrappers for C and Java
• adding error messages in strings.json

Explanation on the used architecture for the docker

Since I changed the architecture used to handle the command, here's a schema explaining the new architecture.
It has been made to easily add a new language, you just need a Dockerfile, a .sh script to run inside the docker and a basic java class, an example can be found for the pythonRunner.

This has been made to allow other contributors to add languages such rust,oz,erlang,haskell,C++,R,ruby,elixir,swift,go,javascript and of course holy C

Functionnalities,

The command can handle multiple code at once, but has a number of maximum 10 processes, it can be changed but with 5 python infinite loop doing nothing, my computer is already in trouble so it should be enough. It is possible to go over the limit but after that there is no guarantee that the docker id's wont get mixed up.

It works in python,c,java and runs the submitted code in safe docker, for having the feature tested, it can't get access to the token in any way.

Running it in docker offers control over the code environment and allow to add libraries like numpy.

For java and c, it is possible to run the code without having to define verbose classes and main function, printf("hello"); would work.

Limitations

Well running side dockers is power intensive, for the purpose of the command, I don't expect it to be used that much and make the bot crash.
I could run about 6 python loops in parallel without any crash or trouble only the fans taking off.

The wrappers are usefull but have some limitations, if a class is defined but no Main class, the program won't be able to run it since it will detect the presence of a class but can't wrap the whole inside a Main class, same goes for function, having a function defined like
int test(){return 0;} without main won't be wrapped but an error message will be thrown in any of these cases.

The output in terms of discord message is big and for a fast paced channel won't fit good. It would be preferable if there was a channel dedicated to that or bot-commands to allow spam, as it can send message up to 2000 chars

Another limitation is the usage of print("\n") it can't be done since it will be written to main.py inside the docker to

print("
")

to avoid this, the user simply need to use print("\\n")

Usage case

It might be usefull to remind the use case, it was intended to help student trying to learn language for lepl1401, lepl1402, lepl1503 by being able to quickly and easily debug together for example by printing wrong types being assigned etc...

[Hokkaydo]

On the paper, I love how you implemented all of that. I'll check on my side to see if everything works, but you did an insane job.
For the "\n" problem, is it possible to detect and escape such inputs before passing the code to docker?

Concerning your previous comment, you asked

Why do i need to have java installed to firstly build the project and then build the docker image ? To run it on WSL i have to install and setup java and its kinda fastidious, why don't you build the project inside of the docker container during the image build?

Indeed I could directly build the jar in the Dockerfile. But anyway it would require you to have Java installed to build the image

[Hokkaydo]

The output of discord messages is big and for a fast-paced channel won't fit well. It would be preferable if there is a channel dedicated to that or bot commands to allow spam, as it can send messages up to 2000 chars

Concerning this, it just came across my mind that you can send "sender-only" replies meaning that he will be the only one able to see (and discard) the message, hence not polluting the channel

[emecdelam]

Regarding your suggestion of sender-only, this only works for InteractionEvent replies, while currently the answer gets separated in 3, the interaction response to the slash command with a timestamp to evaluate the running time of the command, then, when the dockers gets destroyed it sends the original code (to know what the command has been called with, I personnaly find it usefull while explaining code to have the answer and the code). And finally the result of the code directly from the docker ( with errors and exit code ).

So right now I can only get one response to be sent as ephemeral (user-only). So how I see it is either I completely change how the replies are handled to only send one message that gets updated or to send in dm, but to be honest I'm not found of any of these alternatives as it's against the purpose of the feature : explaining and debugging code wich usually is done with 2 people. I dont think the feature will be a huge succes and be that much used, I see it more as an occasional usefull feature (the idea appeared when I wanted to help for a simple type error). I just suggested that there should be a place to send it so if the sent code is wrong, it doesn't pollute the channel with long error messages. Another reason that I dont like
the first approach is that it may force the response to be a file (that has to be downloaded to get the full response) when sometimes (most likely case for verbose language ig java) it could actually be sent trough 2 messages.

To conclude, I can implement it (more likely the first option) but I personally dont think it would be that much of an use for the contraints it presents (1 file instead of 2 messages) and the overrall utility and goal of the command.

Anyway it's up to you to decide but I would rather keep it as is.