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

x/simulation: (*StandardLogWriter).PrintLogs() creates files but never closes them, but also the os.Create should check that error #6784

Closed
4 tasks
odeke-em opened this issue Jul 19, 2020 · 1 comment · Fixed by #8217

Comments

@odeke-em
Copy link
Collaborator

Problems

  • Noticed via an audit, the code from (*StandardLogWriter).PrintLogs() requests for *os.File but never closes it

    func (lw *StandardLogWriter) PrintLogs() {
    f := createLogFile()
    for i := 0; i < len(lw.OpEntries); i++ {
    writeEntry := fmt.Sprintf("%s\n", (lw.OpEntries[i]).MustMarshal())
    _, err := f.WriteString(writeEntry)
    if err != nil {
    panic("Failed to write logs to file")
    }
    }
    }

  • We should be checking that the error returned after os.Create(filepath) checks an error and panics if non-nil

    f, _ = os.Create(filePath)
    fmt.Printf("Logs to writing to %s\n", filePath)
    return f

We need to fix that code in the 2 places as

// PrintLogs - print the logs to a simulation file
func (lw *StandardLogWriter) PrintLogs() {
        f := createLogFile()
        defer  f.Close()

        for i := 0; i < len(lw.OpEntries); i++ {
                writeEntry := fmt.Sprintf("%s\n", (lw.OpEntries[i]).MustMarshal())
                _, err := f.WriteString(writeEntry)

                if err != nil {
                        panic("Failed to write logs to file")
                }
        }
}

func createLogFile() *os.File {
        fileName := fmt.Sprintf("%s.log", time.Now().Format("2006-01-02_15:04:05"))
        folderPath := os.ExpandEnv("$HOME/.simapp/simulations")
        filePath := path.Join(folderPath, fileName)

        if err := os.MkdirAll(folderPath, os.ModePerm); err != nil {
                panic(err)
        }

        f, err := os.Create(filePath)
        if err != nil {
                panic(err)
        }
        fmt.Printf("Logs to writing to %s\n", filePath)

        return f
}

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@odeke-em
Copy link
Collaborator Author

Even better, perhaps we need to remove that harded "/" separators to make it

folderPath := filepath.Join(os.ExpandEnv("$HOME"), ".simapp", "simulations")

@alexanderbez alexanderbez added this to the v0.41 milestone Jul 20, 2020
@mergify mergify bot closed this as completed in #8217 Dec 22, 2020
alessio pushed a commit that referenced this issue Dec 22, 2020
From: #8217
Closes: #6784
Thanks: @PrathyushaLakkireddy for the original patch.

Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Anil Kumar Kammari <anil@vitwit.com>
alessio pushed a commit that referenced this issue Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants