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!

asked May 19, 2018 at 7:29
\$\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)
}
answered May 19, 2018 at 11:30
\$\endgroup\$
1
  • \$\begingroup\$ Exactly what I was looking for, thank-you! Off to do some refactoring. \$\endgroup\$ Commented May 19, 2018 at 11:32

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.