From 0165352a63cb2f78871c2e72d4f2fd4f111637c3 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Tue, 14 Apr 2026 11:29:56 +0300 Subject: fix(report): propagate Write errors; shorten query/upload paths (ask 44) - Return io.WriteString errors from WriteReports via writeReportString - Split ParseReportQuery into small URL parsing helpers - Extract serveUploadPut and uploadAuthorized from upload handler closure - Build report header with strings.Builder Made-with: Cursor --- internal/daemon/upload.go | 105 +++++++++++++++++++---------------- internal/goprecords/report.go | 25 +++++++-- internal/goprecords/report_config.go | 89 +++++++++++++++++------------ internal/goprecords/report_format.go | 22 ++++---- 4 files changed, 144 insertions(+), 97 deletions(-) (limited to 'internal') diff --git a/internal/daemon/upload.go b/internal/daemon/upload.go index be6c6d9..71bffd0 100644 --- a/internal/daemon/upload.go +++ b/internal/daemon/upload.go @@ -24,57 +24,66 @@ var uploadKinds = map[string]string{ func uploadHandler(statsDir string, store *authkeys.Store) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPut { - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) - return - } - host, kind, ok := parseUploadPath(r.URL.Path) - if !ok { - http.Error(w, "bad path", http.StatusBadRequest) - return - } - ext, ok := uploadKinds[kind] - if !ok { - http.Error(w, "unknown file kind", http.StatusBadRequest) - return - } - ctx := r.Context() - nKeys, err := store.KeyCount(ctx) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - if nKeys > 0 { - tok, ok := parseBearer(r.Header.Get("Authorization")) - if !ok || tok == "" { - w.Header().Set("WWW-Authenticate", `Bearer realm="upload"`) - http.Error(w, "unauthorized", http.StatusUnauthorized) - return - } - valid, err := store.Verify(ctx, host, tok) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - if !valid { - http.Error(w, "forbidden", http.StatusForbidden) - return - } - } - rel := host + ext - target := filepath.Join(statsDir, rel) - if !fileUnderDir(statsDir, target) { - http.Error(w, "bad path", http.StatusBadRequest) - return - } - if err := writeUploadBody(target, r.Body); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - w.WriteHeader(http.StatusNoContent) + serveUploadPut(w, r, statsDir, store) }) } +func serveUploadPut(w http.ResponseWriter, r *http.Request, statsDir string, store *authkeys.Store) { + if r.Method != http.MethodPut { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + host, kind, ok := parseUploadPath(r.URL.Path) + if !ok { + http.Error(w, "bad path", http.StatusBadRequest) + return + } + ext, ok := uploadKinds[kind] + if !ok { + http.Error(w, "unknown file kind", http.StatusBadRequest) + return + } + ctx := r.Context() + nKeys, err := store.KeyCount(ctx) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + if nKeys > 0 && !uploadAuthorized(ctx, w, store, host, r.Header.Get("Authorization")) { + return + } + rel := host + ext + target := filepath.Join(statsDir, rel) + if !fileUnderDir(statsDir, target) { + http.Error(w, "bad path", http.StatusBadRequest) + return + } + if err := writeUploadBody(target, r.Body); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + w.WriteHeader(http.StatusNoContent) +} + +func uploadAuthorized(ctx context.Context, w http.ResponseWriter, store *authkeys.Store, host, authz string) bool { + tok, ok := parseBearer(authz) + if !ok || tok == "" { + w.Header().Set("WWW-Authenticate", `Bearer realm="upload"`) + http.Error(w, "unauthorized", http.StatusUnauthorized) + return false + } + valid, err := store.Verify(ctx, host, tok) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return false + } + if !valid { + http.Error(w, "forbidden", http.StatusForbidden) + return false + } + return true +} + func parseUploadPath(path string) (host, kind string, ok bool) { const prefix = "/upload/" if !strings.HasPrefix(path, prefix) { diff --git a/internal/goprecords/report.go b/internal/goprecords/report.go index 6724c47..11c809a 100644 --- a/internal/goprecords/report.go +++ b/internal/goprecords/report.go @@ -62,9 +62,13 @@ func WriteReports(w io.Writer, aggregates *Aggregates, cfg ReportConfig) error { return fmt.Errorf("Category %s only supports: Boots, Uptime, Score", cfg.Category) } if cfg.Category == CategoryHost { - io.WriteString(w, NewHostReporter(aggregates, cfg.Limit, cfg.Metric, cfg.OutputFormat, 1).Report()) + if err := writeReportString(w, NewHostReporter(aggregates, cfg.Limit, cfg.Metric, cfg.OutputFormat, 1).Report()); err != nil { + return err + } } else { - io.WriteString(w, NewReporter(aggregates, cfg.Category, cfg.Limit, cfg.Metric, cfg.OutputFormat, 1).Report()) + if err := writeReportString(w, NewReporter(aggregates, cfg.Category, cfg.Limit, cfg.Metric, cfg.OutputFormat, 1).Report()); err != nil { + return err + } } return nil } @@ -82,15 +86,26 @@ func WriteReports(w io.Writer, aggregates *Aggregates, cfg ReportConfig) error { continue } if c == CategoryHost { - io.WriteString(w, NewHostReporter(aggregates, cfg.Limit, m, cfg.OutputFormat, headerIndent).Report()) + if err := writeReportString(w, NewHostReporter(aggregates, cfg.Limit, m, cfg.OutputFormat, headerIndent).Report()); err != nil { + return err + } } else { - io.WriteString(w, NewReporter(aggregates, c, cfg.Limit, m, cfg.OutputFormat, headerIndent).Report()) + if err := writeReportString(w, NewReporter(aggregates, c, cfg.Limit, m, cfg.OutputFormat, headerIndent).Report()); err != nil { + return err + } + } + if err := writeReportString(w, "\n"); err != nil { + return err } - io.WriteString(w, "\n") } return nil } +func writeReportString(w io.Writer, s string) error { + _, err := io.WriteString(w, s) + return err +} + type Reporter interface { Report() string } diff --git a/internal/goprecords/report_config.go b/internal/goprecords/report_config.go index 8cd8f8d..8dc43d8 100644 --- a/internal/goprecords/report_config.go +++ b/internal/goprecords/report_config.go @@ -73,51 +73,29 @@ func (rf *ReportFlags) Parse() (ReportConfig, error) { // output-format, all, include-kernel, stats-order). It also accepts Category, // Metric, and OutputFormat as alternate keys (same values as the CLI). func ParseReportQuery(q url.Values) (ReportConfig, error) { - catStr := firstQuery(q, "category", "Category") - if catStr == "" { - catStr = "Host" - } - cat, err := ParseCategory(catStr) + cat, err := parseReportQueryCategory(q) if err != nil { return ReportConfig{}, err } - metStr := firstQuery(q, "metric", "Metric") - if metStr == "" { - metStr = "Uptime" - } - met, err := ParseMetric(metStr) + met, err := parseReportQueryMetric(q) if err != nil { return ReportConfig{}, err } - limit := uint(20) - if ls := q.Get("limit"); ls != "" { - v, err := strconv.ParseUint(ls, 10, 32) - if err != nil { - return ReportConfig{}, fmt.Errorf("invalid limit %q", ls) - } - limit = uint(v) - } - outStr := firstQuery(q, "output-format", "OutputFormat") - if outStr == "" { - outStr = "Plaintext" + limit, err := parseReportQueryLimit(q) + if err != nil { + return ReportConfig{}, err } - outFmt, err := ParseOutputFormat(outStr) + outFmt, err := parseReportQueryOutputFormat(q) if err != nil { return ReportConfig{}, err } - all := false - if v := q.Get("all"); v != "" { - all, err = parseQueryBool(v) - if err != nil { - return ReportConfig{}, err - } + all, err := parseReportQueryOptionalBool(q, "all") + if err != nil { + return ReportConfig{}, err } - includeKernel := false - if v := q.Get("include-kernel"); v != "" { - includeKernel, err = parseQueryBool(v) - if err != nil { - return ReportConfig{}, err - } + includeKernel, err := parseReportQueryOptionalBool(q, "include-kernel") + if err != nil { + return ReportConfig{}, err } return ReportConfig{ Category: cat, @@ -130,6 +108,49 @@ func ParseReportQuery(q url.Values) (ReportConfig, error) { }, nil } +func parseReportQueryCategory(q url.Values) (Category, error) { + s := firstQuery(q, "category", "Category") + if s == "" { + s = "Host" + } + return ParseCategory(s) +} + +func parseReportQueryMetric(q url.Values) (Metric, error) { + s := firstQuery(q, "metric", "Metric") + if s == "" { + s = "Uptime" + } + return ParseMetric(s) +} + +func parseReportQueryLimit(q url.Values) (uint, error) { + if ls := q.Get("limit"); ls != "" { + v, err := strconv.ParseUint(ls, 10, 32) + if err != nil { + return 0, fmt.Errorf("invalid limit %q", ls) + } + return uint(v), nil + } + return 20, nil +} + +func parseReportQueryOutputFormat(q url.Values) (OutputFormat, error) { + s := firstQuery(q, "output-format", "OutputFormat") + if s == "" { + s = "Plaintext" + } + return ParseOutputFormat(s) +} + +func parseReportQueryOptionalBool(q url.Values, key string) (bool, error) { + v := q.Get(key) + if v == "" { + return false, nil + } + return parseQueryBool(v) +} + func firstQuery(q url.Values, keys ...string) string { for _, k := range keys { if v := q.Get(k); v != "" { diff --git a/internal/goprecords/report_format.go b/internal/goprecords/report_format.go index 7ec66b3..0d5e4dd 100644 --- a/internal/goprecords/report_format.go +++ b/internal/goprecords/report_format.go @@ -101,29 +101,31 @@ func (r reportBuilder) buildBorder(countW, nameW, valueW, lastKernelW int, hasLa } func (r reportBuilder) buildReportHeader(countW, nameW, valueW, lastKernelW int, hasLastKernel bool, border string, outputFormat OutputFormat) string { - var h string + var b strings.Builder if outputFormat == FormatMarkdown || outputFormat == FormatGemtext { - h = strings.Repeat("#", int(r.headerIndent)) + " " + b.WriteString(strings.Repeat("#", int(r.headerIndent))) + b.WriteString(" ") } - h += fmt.Sprintf("Top %d %s's by %s\n\n", r.limit, r.metric, r.category) + b.WriteString(fmt.Sprintf("Top %d %s's by %s\n\n", r.limit, r.metric, r.category)) desc := MetricDescription(r.metric) lineLimit := len(border) if outputFormat == FormatPlaintext && lineLimit > 0 && len(desc) > lineLimit-1 { desc = " " + wordWrap(desc, lineLimit-1) } - h += desc + "\n\n" + b.WriteString(desc) + b.WriteString("\n\n") if outputFormat == FormatMarkdown || outputFormat == FormatGemtext { - h += "```\n" + b.WriteString("```\n") } - h += border + b.WriteString(border) fmtStr := r.buildFormatStr(countW, nameW, valueW, lastKernelW, hasLastKernel) if hasLastKernel { - h += fmt.Sprintf(fmtStr+"\n", "Pos", r.category.String(), r.metric.String(), "Last Kernel") + b.WriteString(fmt.Sprintf(fmtStr+"\n", "Pos", r.category.String(), r.metric.String(), "Last Kernel")) } else { - h += fmt.Sprintf(fmtStr+"\n", "Pos", r.category.String(), r.metric.String()) + b.WriteString(fmt.Sprintf(fmtStr+"\n", "Pos", r.category.String(), r.metric.String())) } - h += border - return h + b.WriteString(border) + return b.String() } func (r reportBuilder) buildFormatStr(countW, nameW, valueW, lastKernelW int, hasLastKernel bool) string { -- cgit v1.2.3