diff --git a/internal/bridge/bridge.go b/internal/bridge/bridge.go index d294f62b..94abb4d6 100644 --- a/internal/bridge/bridge.go +++ b/internal/bridge/bridge.go @@ -19,7 +19,6 @@ package bridge import ( - "context" "fmt" "strconv" "time" @@ -126,19 +125,6 @@ func (b *Bridge) heartbeat() { } } -// ReportBug reports a new bug from the user. -func (b *Bridge) ReportBug(osType, osVersion, description, accountName, address, emailClient string) error { - return b.clientManager.ReportBug(context.Background(), pmapi.ReportBugReq{ - OS: osType, - OSVersion: osVersion, - Browser: emailClient, - Title: "[Bridge] Bug", - Description: description, - Username: accountName, - Email: address, - }) -} - // GetUpdateChannel returns currently set update channel. func (b *Bridge) GetUpdateChannel() updater.UpdateChannel { return updater.UpdateChannel(b.settings.Get(settings.UpdateChannelKey)) diff --git a/internal/bridge/bug_report.go b/internal/bridge/bug_report.go new file mode 100644 index 00000000..0b2fd576 --- /dev/null +++ b/internal/bridge/bug_report.go @@ -0,0 +1,199 @@ +// Copyright (c) 2021 Proton Technologies AG +// +// This file is part of ProtonMail Bridge. +// +// ProtonMail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// ProtonMail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with ProtonMail Bridge. If not, see . + +package bridge + +import ( + "archive/zip" + "bytes" + "context" + "errors" + "io" + "io/ioutil" + "os" + "path/filepath" + "sort" + + "github.com/ProtonMail/proton-bridge/internal/logging" + "github.com/ProtonMail/proton-bridge/pkg/pmapi" +) + +const MaxAttachmentSize = 7 * 1024 * 1024 // 7 MB total limit +const MaxCompressedFilesCount = 6 + +var ErrSizeTooLarge = errors.New("file is too big") + +// ReportBug reports a new bug from the user. +func (b *Bridge) ReportBug(osType, osVersion, description, accountName, address, emailClient string, attachLogs bool) error { + report := pmapi.ReportBugReq{ + OS: osType, + OSVersion: osVersion, + Browser: emailClient, + Title: "[Bridge] Bug", + Description: description, + Username: accountName, + Email: address, + } + + if attachLogs { + logs, err := b.getMatchingLogs( + func(filename string) bool { + return logging.MatchLogName(filename) && !logging.MatchStackTraceName(filename) + }, + ) + if err != nil { + log.WithError(err).Error("Can't get log files list") + } + crashes, err := b.getMatchingLogs( + func(filename string) bool { + return logging.MatchLogName(filename) && logging.MatchStackTraceName(filename) + }, + ) + if err != nil { + log.WithError(err).Error("Can't get crash files list") + } + + var matchFiles []string + + matchFiles = append(matchFiles, logs[max(0, len(logs)-(MaxCompressedFilesCount/2)):]...) + matchFiles = append(matchFiles, crashes[max(0, len(crashes)-(MaxCompressedFilesCount/2)):]...) + + archive, err := zipFiles(matchFiles) + if err != nil { + log.WithError(err).Error("Can't zip logs and crashes") + } + + if archive != nil { + report.AddAttachment("logs.zip", "application/zip", archive) + } + } + + return b.clientManager.ReportBug(context.Background(), report) +} + +func max(a, b int) int { + if a > b { + return a + } + return b +} + +func (b *Bridge) getMatchingLogs(filenameMatchFunc func(string) bool) (filenames []string, err error) { + logsPath, err := b.locations.ProvideLogsPath() + if err != nil { + return nil, err + } + + files, err := ioutil.ReadDir(logsPath) + if err != nil { + return nil, err + } + + var matchFiles []string + + for _, file := range files { + if filenameMatchFunc(file.Name()) { + matchFiles = append(matchFiles, filepath.Join(logsPath, file.Name())) + } + } + sort.Strings(matchFiles) // Sorted by timestamp: oldest first. + + return matchFiles, nil +} + +type LimitedBuffer struct { + capacity int + buf *bytes.Buffer +} + +func NewLimitedBuffer(capacity int) *LimitedBuffer { + return &LimitedBuffer{ + capacity: capacity, + buf: bytes.NewBuffer(make([]byte, 0, capacity)), + } +} + +func (b *LimitedBuffer) Write(p []byte) (n int, err error) { + if len(p)+b.buf.Len() > b.capacity { + return 0, ErrSizeTooLarge + } + + return b.buf.Write(p) +} + +func (b *LimitedBuffer) Read(p []byte) (n int, err error) { + return b.buf.Read(p) +} + +func zipFiles(filenames []string) (io.Reader, error) { + if len(filenames) == 0 { + return nil, nil + } + + buf := NewLimitedBuffer(MaxAttachmentSize) + + w := zip.NewWriter(buf) + defer w.Close() //nolint[errcheck] + + for _, file := range filenames { + err := addFileToZip(file, w) + if err != nil { + return nil, err + } + } + + if err := w.Close(); err != nil { + return nil, err + } + + return buf, nil +} + +func addFileToZip(filename string, writer *zip.Writer) error { + fileReader, err := os.Open(filepath.Clean(filename)) + if err != nil { + return err + } + defer fileReader.Close() //nolint[errcheck] + + fileInfo, err := fileReader.Stat() + if err != nil { + return err + } + + header, err := zip.FileInfoHeader(fileInfo) + if err != nil { + return err + } + + header.Method = zip.Deflate + header.Name = filepath.Base(filename) + + fileWriter, err := writer.CreateHeader(header) + if err != nil { + return err + } + + _, err = io.Copy(fileWriter, fileReader) + if err != nil { + return err + } + + err = fileReader.Close() + + return err +} diff --git a/internal/bridge/types.go b/internal/bridge/types.go index 1077acf9..fe0a541f 100644 --- a/internal/bridge/types.go +++ b/internal/bridge/types.go @@ -26,6 +26,7 @@ import ( type Locator interface { Clear() error ClearUpdates() error + ProvideLogsPath() (string, error) } type CacheProvider interface { diff --git a/internal/frontend/qt/frontend_help.go b/internal/frontend/qt/frontend_help.go index e105e0f4..588375e2 100644 --- a/internal/frontend/qt/frontend_help.go +++ b/internal/frontend/qt/frontend_help.go @@ -19,6 +19,8 @@ package qt +import "github.com/therecipe/qt/core" + func (f *FrontendQt) setVersion() { f.qml.SetVersion(f.programVersion) } @@ -41,5 +43,21 @@ func (f *FrontendQt) setCurrentEmailClient() { } func (f *FrontendQt) reportBug(description, address, emailClient string, includeLogs bool) { - //TODO + defer f.qml.ReportBugFinished() + + if err := f.bridge.ReportBug( + core.QSysInfo_ProductType(), + core.QSysInfo_PrettyProductName(), + description, + "Unknown account", + address, + emailClient, + includeLogs, + ); err != nil { + f.log.WithError(err).Error("Failed to report bug") + f.qml.BugReportSendError() + return + } + + f.qml.BugReportSendSuccess() } diff --git a/internal/frontend/types/types.go b/internal/frontend/types/types.go index d1c63308..31a7fe75 100644 --- a/internal/frontend/types/types.go +++ b/internal/frontend/types/types.go @@ -74,7 +74,7 @@ type User interface { type Bridger interface { UserManager - ReportBug(osType, osVersion, description, accountName, address, emailClient string) error + ReportBug(osType, osVersion, description, accountName, address, emailClient string, attachLogs bool) error AllowProxy() DisallowProxy() EnableCache() error diff --git a/internal/logging/clear.go b/internal/logging/clear.go index e228145b..13c3ee4d 100644 --- a/internal/logging/clear.go +++ b/internal/logging/clear.go @@ -26,7 +26,7 @@ import ( "github.com/sirupsen/logrus" ) -func clearLogs(logDir string, maxLogs int) error { +func clearLogs(logDir string, maxLogs int, maxCrashes int) error { files, err := ioutil.ReadDir(logDir) if err != nil { return err @@ -36,8 +36,8 @@ func clearLogs(logDir string, maxLogs int) error { var crashesWithPrefix []string for _, file := range files { - if matchLogName(file.Name()) { - if matchStackTraceName(file.Name()) { + if MatchLogName(file.Name()) { + if MatchStackTraceName(file.Name()) { crashesWithPrefix = append(crashesWithPrefix, file.Name()) } else { logsWithPrefix = append(logsWithPrefix, file.Name()) @@ -46,7 +46,7 @@ func clearLogs(logDir string, maxLogs int) error { // Older versions of Bridge stored logs in subfolders for each version. // That also has to be cleared and the functionality can be removed after some time. if file.IsDir() { - if err := clearLogs(filepath.Join(logDir, file.Name()), maxLogs); err != nil { + if err := clearLogs(filepath.Join(logDir, file.Name()), maxLogs, maxCrashes); err != nil { return err } } else { @@ -56,7 +56,7 @@ func clearLogs(logDir string, maxLogs int) error { } removeOldLogs(logDir, logsWithPrefix, maxLogs) - removeOldLogs(logDir, crashesWithPrefix, maxLogs) + removeOldLogs(logDir, crashesWithPrefix, maxCrashes) return nil } @@ -76,10 +76,10 @@ func removeOldLogs(logDir string, filenames []string, maxLogs int) { func removeLog(logDir, filename string) { // We need to be sure to delete only log files. // Directory with logs can also contain other files. - if !matchLogName(filename) { + if !MatchLogName(filename) { return } - if err := os.RemoveAll(filepath.Join(logDir, filename)); err != nil { - logrus.WithError(err).Error("Failed to remove old logs") + if err := os.Remove(filepath.Join(logDir, filename)); err != nil { + logrus.WithError(err).Error("Failed to remove", filepath.Join(logDir, filename)) } } diff --git a/internal/logging/crash.go b/internal/logging/crash.go index cc3c8a2c..6b8d1762 100644 --- a/internal/logging/crash.go +++ b/internal/logging/crash.go @@ -57,6 +57,6 @@ func getStackTraceName(version, revision string) string { return fmt.Sprintf("v%v_%v_crash_%v.log", version, revision, time.Now().Unix()) } -func matchStackTraceName(name string) bool { +func MatchStackTraceName(name string) bool { return regexp.MustCompile(`^v.*_crash_.*\.log$`).MatchString(name) } diff --git a/internal/logging/logging.go b/internal/logging/logging.go index 62b886e8..c19c19f1 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -31,12 +31,17 @@ import ( ) const ( - // MaxLogSize defines the maximum log size we should permit. - // Zendesk has a file size limit of 20MB. When the last N log files are zipped, - // it should fit under 20MB. So here we permit up to 10MB (most files are a few hundred kB). - MaxLogSize = 10 * 2 << 20 + // MaxLogSize defines the maximum log size we should permit: 5 MB + // + // The Zendesk limit for an attachement is 50MB and this is what will + // be allowed via the API. However, if that fails for some reason, the + // fallback is sending the report via email, which has a limit of 10mb + // total or 7MB per file. Since we can produce up to 6 logs, and we + // compress all the files (avarage compression - 80%), we need to have + // a limit of 30MB total before compression, hence 5MB per log file. + MaxLogSize = 5 * 1024 * 1024 - // MaxLogs defines how many old log files should be kept. + // MaxLogs defines how many log files should be kept. MaxLogs = 3 ) @@ -56,7 +61,8 @@ func Init(logsPath string) error { }) rotator, err := NewRotator(MaxLogSize, func() (io.WriteCloser, error) { - if err := clearLogs(logsPath, MaxLogs); err != nil { + // Leaving MaxLogs-1 since new log file will be opened right away. + if err := clearLogs(logsPath, MaxLogs-1, MaxLogs); err != nil { return nil, err } @@ -91,6 +97,6 @@ func getLogName(version, revision string) string { return fmt.Sprintf("v%v_%v_%v.log", version, revision, time.Now().Unix()) } -func matchLogName(name string) bool { +func MatchLogName(name string) bool { return regexp.MustCompile(`^v.*\.log$`).MatchString(name) } diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go index df1e0bbf..e6193448 100644 --- a/internal/logging/logging_test.go +++ b/internal/logging/logging_test.go @@ -36,7 +36,7 @@ func TestClearLogs(t *testing.T) { require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "v2_12.log"), []byte("Hello"), 0755)) require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "v2_13.log"), []byte("Hello"), 0755)) - require.NoError(t, clearLogs(dir, 3)) + require.NoError(t, clearLogs(dir, 3, 0)) checkFileNames(t, dir, []string{ "other.log", "v1_11.log", diff --git a/pkg/pmapi/manager_report.go b/pkg/pmapi/manager_report.go index 6d08f935..c14f40c3 100644 --- a/pkg/pmapi/manager_report.go +++ b/pkg/pmapi/manager_report.go @@ -38,7 +38,7 @@ func (m *manager) ReportBug(ctx context.Context, rep ReportBugReq) error { r := m.r(ctx).SetMultipartFormData(rep.GetMultipartFormData()) for _, att := range rep.Attachments { - r = r.SetMultipartField(att.name, att.filename, "application/octet-stream", att.body) + r = r.SetMultipartField(att.name, att.name, att.mime, att.body) } if _, err := wrapNoConnection(r.Post("/reports/bug")); err != nil { diff --git a/pkg/pmapi/manager_report_types.go b/pkg/pmapi/manager_report_types.go index e00d2a23..4bb21f43 100644 --- a/pkg/pmapi/manager_report_types.go +++ b/pkg/pmapi/manager_report_types.go @@ -29,8 +29,8 @@ const ( ) type reportAtt struct { - name, filename string - body io.Reader + name, mime string + body io.Reader } // ReportBugReq stores data for report. @@ -56,8 +56,8 @@ type ReportBugReq struct { } // AddAttachment to report. -func (rep *ReportBugReq) AddAttachment(name, filename string, r io.Reader) { - rep.Attachments = append(rep.Attachments, reportAtt{name: name, filename: filename, body: r}) +func (rep *ReportBugReq) AddAttachment(name, mime string, r io.Reader) { + rep.Attachments = append(rep.Attachments, reportAtt{name: name, mime: mime, body: r}) } func (rep *ReportBugReq) GetMultipartFormData() map[string]string {