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

Question: Debug or non-debug build for serious shell scripting? #118

Closed
joshgoebel opened this issue Aug 30, 2021 · 5 comments
Closed

Question: Debug or non-debug build for serious shell scripting? #118

joshgoebel opened this issue Aug 30, 2021 · 5 comments

Comments

@joshgoebel
Copy link
Contributor

joshgoebel commented Aug 30, 2021

In working on Process.chdir it was brought to my attention how fragile passing the wrong arguments can be with a production Wren build:

$ bin/wrenc
 -"\//
  \_/
wrenc v0.2.91 (wren v0.4.0) (based on wren-cli@9c6b6933722)
> import "os" for Process
> Process.chdir(["project","make.mac"])
Runtime error: Cannot change directory.
> Process.chdir(1.exp)
fish: Job 1, 'bin/wrenc' terminated by signal SIGSEGV (Address boundary error)

We don't seem to validate most arguments (looking at Wren CLI codebase) other than the ASSERT guard rails on all the built in getWrenString, etc. helpers... so I'm wondering... should the advice for people wishing to use Wren as a serious tool for shell scripting be to always use a debug build - for the additional security/error protection guarantees?

@CrazyInfin8
Copy link

It appears wren-cli does validate parameters though not on the C side.

For example, opening a file:

// io.wren
foreign class File {
    ...
    static openWithFlags(path, flags) {
        ensureString_(path)
        ensureInt_(flags, "Flags")
        var fd = Scheduler.await_ { open_(path, flags, Fiber.current) }
        return new_(fd)
    }
    ...
    static ensureString_(path) {
        if (!(path is String)) Fiber.abort("Path must be a string.")
    }

    static ensureInt_(value, name) {
        if (!(value is Num)) Fiber.abort("%(name) must be an integer.")
        if (!value.isInteger) Fiber.abort("%(name) must be an integer.")
        if (value < 0) Fiber.abort("%(name) cannot be negative.")
    }
    ...
}

I've always thought of wren-cli as more of a toy or example (much like wren.io/try). I mean since privacy isn't that strong there is nothing stopping a user from doing:

File.open_("junk", "junk", "junk")
File.new_("and more junk")

So in a go-based wren-cli that I am working on, I actually have a publicly available set of modules with classes that wrap around foreign classes in a private module (see here) and a class with static methods meant to validate parameters like so:

class Validate {
    static Num(v, name) {
        if (!(v is Num)) Fiber.abort("Expected 'Num' for '%(name)'.")
    }

    static Int(v, name) {
        if (!(v is Num) || !v.isInteger) Fiber.abort("Expected integer for '%(name)'.")
    }

    static PositiveInt(v, name) {
        if (!(v is Num) || !v.isInteger || v < 0) Fiber.abort("Expected positive integer for '%(name)'.")
    }

    static String(v, name) {
        if (!(v is String)) Fiber.abort("Expected 'String' for '%(name)'")
    }

    static Bool(v, name) {
        if (!(v is bool)) Fiber.abort("Expected 'Bool' for '%(name)'")
    }

    static Fn(v, arity, name) {
        if (!(v is Fn) || v.arity != arity) Fiber.abort("Expected 'Fn' with %(arity) parameters for '%(name)'")
    }

    static Type(v, type, name) {
        if (!(v is type)) Fiber.abort("Expected '%(type)' for '%(name)'")
    }
}

so I can have much safer public classes and modules looking like this:

class File {
    construct openWithFlags(name, flag, perm) {
        Validate.String(name, "name")
        Validate.Int(flag, "flag")
        Validate.Int(perm, "perm")
        import "sys:os" for File // sys:os is only importable by certain built in modules
        _file = File.new(name, flag, perm)
    }
    ...
}

The other benefit to checking on the wren is it's easier (at least I think so) to check that a value is of a certain class type in wren since we have access to the is keyword. For example someone could have a constructor for a Stat class that can optionally take a File object so it can stat that file. we can check that param is File in wren but I don't think it is that easy on the C side

@joshgoebel
Copy link
Contributor Author

So in a go-based wren-cli that I am working on

Link? What's the license for that Validate code? I think we should add it to the CLI or failing that Wren Essentials - seems it's quite useful for building robust APIs and should just be included.

@joshgoebel
Copy link
Contributor Author

joshgoebel commented Aug 31, 2021

Deleted my whole comment, I'm still reading the C code, ugh...

The other benefit to checking on the wren is it's easier (at least I think so) to check that a value is of a certain class type in wren since we have access to the is keyword.

Agree, doing this in Wren makes more sense than C.

@CrazyInfin8
Copy link

Link? What's the license for that Validate code?

Ah I haven't made a repo yet though it should be under the Unlicense license or MIT when I get around to doing it.

@joshgoebel
Copy link
Contributor Author

@CrazyInfin8 I borrowed this as foundation for use with wren-console.

joshgoebel#13 (comment)

Closing this issue as asked and answered.

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

No branches or pull requests

2 participants