2
\$\begingroup\$

I'm working on a new command line application in GO and was hoping I could get feedback on my design pattern, or suggestions on a better one to use. I'm still new to GO, only having used it for a handful of projects, and would value any suggestions to take better advantage of the "GO" way of doing things.

The intent of the application will be to either scan a single domain, or a list of domains and then output results in either a normal format (depending on flags) containing information about vulnerable domains and suggestions, or a file only showing the domains in the initial input that are vulnerable.

main.go

package main

import (
    "flag"
    "fmt"
    "./libprojectstart"
    "os"
)

func Banner() {
    fmt.Printf("%s%sprojectstart - SSL Mass Renegotiation Tester\n%s", 
        libprojectstart.OKBLUE, libprojectstart.BOLD, libprojectstart.RESET)
}

func ParseCmdLine() (state *libprojectstart.State) {
        s := libprojectstart.InitState()

        flag.StringVar(&s.Domain, "d", "", "Domain to scan")
        flag.StringVar(&s.DomainList, "dL", "", "List of domains to scan")
        flag.StringVar(&s.OutputNormal, "oN", "", "File to write normal output to")
        flag.StringVar(&s.OutputDomains, "oD", "", "File to write successful domains to")
        flag.IntVar(&s.Threads, "t", 20, "Number of threads (Default: 20)")
        flag.BoolVar(&s.NoColour, "--no-colour", true, "Don't Use colors in output")
        flag.BoolVar(&s.Silent, "--silent", false, "Output successful scans only")
        flag.BoolVar(&s.Verbose, "v", false, "Verbose output")
        flag.BoolVar(&s.Usage, "h", false, "Display this message")
        flag.Parse()

        return &s
}

func main() {
    state := ParseCmdLine()
    if state.Silent != true {
        Banner()
    }

    if libprojectstart.IsUnchanged(*state) || state.Usage {
        Banner()
        flag.PrintDefaults()
        os.Exit(1)
    }


    if validated, error := libprojectstart.ValidateState(state); validated != true {
        fmt.Printf("%s[!] %s%s", libprojectstart.FAIL, libprojectstart.RESET, error)
        flag.PrintDefaults()
        os.Exit(1)
    }
}

state.go

package libprojectstart

// Contains state read in from the command line
type State struct {
    Domain          string  // Domain to check for
    DomainList      string  // File location for a list of domains
    OutputNormal    string  // File to output in normal format
    OutputDomains   string  // File to output domains only to
    Verbose         bool    // Verbose prints, incl. Debug information
    Threads         int     // Number of threads to use
    NoColour        bool    // Strip colour from output
    Silent          bool    // Output domains only
    Usage           bool    // Print usage information
}

func InitState() (s State) {
    return State { "", "", "", "", false, 20, false, false, false }
}

func ValidateState(s *State) (result bool, error string) {
    if s.Domain == "" && s.DomainList == "" {
        return false, "You must specify either a domain or list of domains to test"
    }

    return true, ""
}

func IsUnchanged(s State) bool {
    return s == InitState()
}

colour.go

package libprojectstart

var (
    HEADER      = "\033[95m"
    OKBLUE      = "\033[94m"
    OKGREEN     = "\033[92m"
    WARNING     = "\033[93m"
    FAIL        = "\033[91m"
    BOLD        = "\033[1m"
    UNDERLINE   = "\033[4m"
    RESET       = "\033[0m"
)

The intent of this application is that the command line options, as well as the feedback being printed will grow significantly over time. I've aimed to demonstrate some of that error checking in ValidateState() as that function will grow with the project, as will the printing of colours. Open to any and all suggestions for improvements!

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Validation logic

When checking whether the program should print the usage message or not, I find it an unusual approach to say "is the state (of initial parameter values) unchanged?" It would be more natural to check facts more directly. For example in this program the only input you really require is one or more domain names. Then that should be the determining factor for displaying the usage message: "did the user specify a domain name?" Direct, straightforward, to the point.

Naming

The current names are not very intuitive:

  • State is state of what? It's the command line parameters. Something like CmdParams would be better.
  • Banner is a noun, so it sounds like an object, not something that takes some action. PrintBanner would be better.
  • Names starting with capital letters are exported (~ public API) in go. Many if not all names in the posted could be private, so their names should start with lowercase.
  • Instead of InitX, when creating a new instance of X, the name NewX is more common.

Performance

Every time you evaluate s == InitState(), InitState() creates a new instance to perform the comparison. It would be enough to have just one such instance as a frame of reference, created as a var at top-level scope, no need to create a new instance every time.

Style

Many of the functions specify a name for the returned value, but don't actually use it in the function body. So those optional names can be dropped.


Instead of this:

return State { "", "", "", "", false, 20, false, false, false }

You could take advantage of the default zero-values of strings and booleans, and write simpler like this:

return State{Threads: 20}

In my experience, negative boolean variables such as NoColour are often confusing in practice. Inverting the meaning and using !Colour tends to work better.

Use short boolean conditions

Instead of if state.Silent != true, you can write shorter as if !state.Silent.

Banner printed twice

When the script is called without parameters, the banner will be printed twice. Swap these two statements to avoid that.

if state.Silent != true {
    Banner()
}

if libprojectstart.IsUnchanged(*state) || state.Usage {
    Banner()
    flag.PrintDefaults()
    os.Exit(1)
}
\$\endgroup\$
1
  • \$\begingroup\$ Exactly what I was looking for, thank-you! Off to do some refactoring. \$\endgroup\$
    – Michael A
    May 19, 2018 at 11:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.