diff --git a/frontend/src/pages/settings/NotifyTimeField.tsx b/frontend/src/pages/settings/NotifyTimeField.tsx index ea7d2c8ed..b073a9181 100644 --- a/frontend/src/pages/settings/NotifyTimeField.tsx +++ b/frontend/src/pages/settings/NotifyTimeField.tsx @@ -3,11 +3,8 @@ import { useTranslation } from 'react-i18next'; import { Input, InputNumber, Select, Space } from 'antd'; import { onNumber } from '@/utils/onNumber'; -// The notification schedule is fed straight to robfig/cron's AddJob (see -// web.go startTask), which accepts @every , the @hourly/@daily/... -// macros, and full crontab expressions. This builder covers the common cases -// with dropdowns so users don't have to memorise the syntax, while "Custom" -// preserves the raw crontab escape hatch. +// The schedule is parsed by internal/util/cronspec: @every, the @macros, and +// five- or six-field crontab; "Custom" keeps the raw crontab escape hatch. export type Unit = 's' | 'm' | 'h'; export type Macro = '@hourly' | '@daily' | '@weekly' | '@monthly'; export type Mode = 'every' | Macro | 'custom'; @@ -44,11 +41,8 @@ export function composeRunTime(s: RunTime): string { return s.mode; } -// The panel's cron runs with seconds enabled (cron.WithSeconds() in web.go), so -// crontab expressions are 6-field: "second minute hour day month weekday". When -// the user drops into Custom we seed the box with the crontab equivalent of the -// current selection rather than a bare @macro, so they get a real expression to -// edit (and one that the 6-field parser accepts). +// Custom is seeded with the six-field ("second minute hour day month weekday") +// equivalent of the current selection, so the admin edits a real expression. export function toCrontab(s: RunTime): string { switch (s.mode) { case '@hourly': diff --git a/internal/util/cronspec/cronspec.go b/internal/util/cronspec/cronspec.go new file mode 100644 index 000000000..d452682d1 --- /dev/null +++ b/internal/util/cronspec/cronspec.go @@ -0,0 +1,24 @@ +// Package cronspec parses the cron schedules an admin types into the settings. +package cronspec + +import ( + "strings" + + "github.com/robfig/cron/v3" + + "github.com/mhsanaei/3x-ui/v3/internal/util/common" +) + +// The panel scheduler runs WithSeconds(), whose parser rejects five-field crontab, +// so admin schedules are parsed here with the seconds column optional. +var parser = cron.NewParser( + cron.SecondOptional | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor, +) + +func Parse(spec string) (cron.Schedule, error) { + schedule, err := parser.Parse(strings.TrimSpace(spec)) + if err != nil { + return nil, common.NewErrorf("cron schedule <%v> is not valid: %v", spec, err) + } + return schedule, nil +} diff --git a/internal/util/cronspec/cronspec_test.go b/internal/util/cronspec/cronspec_test.go new file mode 100644 index 000000000..e183d6da3 --- /dev/null +++ b/internal/util/cronspec/cronspec_test.go @@ -0,0 +1,47 @@ +package cronspec + +import ( + "strings" + "testing" + "time" +) + +// #1648 upstream: the panel's scheduler runs WithSeconds(), whose parser refused +// a standard five-field crontab, so an admin's "30 8 * * *" never fired. +func TestParseNextFire(t *testing.T) { + from := time.Date(2026, 10, 11, 7, 0, 0, 0, time.UTC) + cases := []struct { + spec string + want time.Time + }{ + {"30 8 * * *", time.Date(2026, 10, 11, 8, 30, 0, 0, time.UTC)}, + {" 15 30 8 * * * ", time.Date(2026, 10, 11, 8, 30, 15, 0, time.UTC)}, + {"@daily", time.Date(2026, 10, 12, 0, 0, 0, 0, time.UTC)}, + {"@every 90m", from.Add(90 * time.Minute)}, + } + for _, c := range cases { + t.Run(c.spec, func(t *testing.T) { + schedule, err := Parse(c.spec) + if err != nil { + t.Fatalf("Parse(%q): %v", c.spec, err) + } + if got := schedule.Next(from); !got.Equal(c.want) { + t.Errorf("Parse(%q).Next = %v, want %v", c.spec, got, c.want) + } + }) + } +} + +func TestParseRejects(t *testing.T) { + for _, spec := range []string{"", " ", "every day", "61 * * * *", "* * * *"} { + t.Run(spec, func(t *testing.T) { + _, err := Parse(spec) + if err == nil { + t.Fatalf("Parse(%q): want error, got nil", spec) + } + if !strings.Contains(err.Error(), "cron schedule") { + t.Errorf("Parse(%q) error = %q, want it to name the cron schedule", spec, err) + } + }) + } +} diff --git a/internal/web/entity/check_valid_test.go b/internal/web/entity/check_valid_test.go index 51751278c..31866910f 100644 --- a/internal/web/entity/check_valid_test.go +++ b/internal/web/entity/check_valid_test.go @@ -78,3 +78,28 @@ func TestCheckValidIPOrCIDRLists(t *testing.T) { t.Errorf("trustedProxyCIDRs error = %v, want it to name the trusted-proxy list and the bad entry", err) } } + +// A schedule the scheduler cannot parse used to be saved anyway, and the report +// or sync job then silently never ran. +func TestCheckValidSchedules(t *testing.T) { + set := map[string]func(*AllSetting, string){ + "tgRunTime": func(s *AllSetting, v string) { s.TgRunTime = v }, + "discordRunTime": func(s *AllSetting, v string) { s.DiscordRunTime = v }, + "ldapSyncCron": func(s *AllSetting, v string) { s.LdapSyncCron = v }, + } + for name, apply := range set { + for _, v := range []string{"", "@daily", "30 8 * * *", "0 30 8 * * *", "@every 1m"} { + s := &AllSetting{WebPort: 2053, SubPort: 2096} + apply(s, v) + if err := s.CheckValid(); err != nil { + t.Errorf("%s=%q: unexpected error %v", name, v, err) + } + } + s := &AllSetting{WebPort: 2053, SubPort: 2096} + apply(s, "every morning") + err := s.CheckValid() + if err == nil || !strings.Contains(err.Error(), name) { + t.Errorf("%s=%q: want an error naming %s, got %v", name, "every morning", name, err) + } + } +} diff --git a/internal/web/entity/entity.go b/internal/web/entity/entity.go index c3ba6aab7..3b2dbf612 100644 --- a/internal/web/entity/entity.go +++ b/internal/web/entity/entity.go @@ -10,6 +10,7 @@ import ( "time" "github.com/mhsanaei/3x-ui/v3/internal/util/common" + "github.com/mhsanaei/3x-ui/v3/internal/util/cronspec" ) type Msg struct { @@ -352,6 +353,19 @@ func (s *AllSetting) CheckValid() error { s.SubClashPath += "/" } + for _, sched := range []struct{ name, spec string }{ + {"tgRunTime", s.TgRunTime}, + {"discordRunTime", s.DiscordRunTime}, + {"ldapSyncCron", s.LdapSyncCron}, + } { + if strings.TrimSpace(sched.spec) == "" { + continue + } + if _, err := cronspec.Parse(sched.spec); err != nil { + return common.NewError(sched.name+":", err) + } + } + if err := checkIPOrCIDRList(s.TrustedProxyCIDRs, "trusted proxy CIDR is not valid:"); err != nil { return err } diff --git a/internal/web/schedule_test.go b/internal/web/schedule_test.go new file mode 100644 index 000000000..7e56c2055 --- /dev/null +++ b/internal/web/schedule_test.go @@ -0,0 +1,36 @@ +package web + +import ( + "testing" + "time" + + "github.com/robfig/cron/v3" +) + +// A five-field crontab used to be refused by the WithSeconds() scheduler, so the +// Telegram/Discord report and LDAP sync were never scheduled at all. +func TestScheduleSettingJob(t *testing.T) { + from := time.Date(2026, 10, 11, 7, 0, 0, 0, time.UTC) + cases := []struct { + name, spec string + want time.Time + }{ + {"five-field crontab", "30 8 * * *", time.Date(2026, 10, 11, 8, 30, 0, 0, time.UTC)}, + {"six-field crontab", "15 30 8 * * *", time.Date(2026, 10, 11, 8, 30, 15, 0, time.UTC)}, + {"empty falls back", " ", time.Date(2026, 10, 12, 0, 0, 0, 0, time.UTC)}, + {"invalid falls back", "every morning", time.Date(2026, 10, 12, 0, 0, 0, 0, time.UTC)}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + sched := cron.New(cron.WithSeconds(), cron.WithLocation(time.UTC)) + id := scheduleSettingJob(sched, "test job", c.spec, "@daily", cron.FuncJob(func() {})) + entry := sched.Entry(id) + if entry.Schedule == nil { + t.Fatalf("spec %q: job was not scheduled", c.spec) + } + if got := entry.Schedule.Next(from); !got.Equal(c.want) { + t.Errorf("spec %q: next run %v, want %v", c.spec, got, c.want) + } + }) + } +} diff --git a/internal/web/web.go b/internal/web/web.go index 8d0b505f8..328843a07 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -24,6 +24,7 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/mtproto" "github.com/mhsanaei/3x-ui/v3/internal/tuic" "github.com/mhsanaei/3x-ui/v3/internal/util/common" + "github.com/mhsanaei/3x-ui/v3/internal/util/cronspec" "github.com/mhsanaei/3x-ui/v3/internal/util/sys" "github.com/mhsanaei/3x-ui/v3/internal/web/controller" "github.com/mhsanaei/3x-ui/v3/internal/web/job" @@ -385,13 +386,9 @@ func (s *Server) startTask(restartXray bool, loc *time.Location) { // LDAP sync scheduling if ldapEnabled, _ := s.settingService.GetLdapEnable(); ldapEnabled { - runtime, err := s.settingService.GetLdapSyncCron() - if err != nil || runtime == "" { - runtime = "@every 1m" - } - j := job.NewLdapSyncJob() + runtime, _ := s.settingService.GetLdapSyncCron() // job has zero-value services with method receivers that read settings on demand - _, _ = s.cron.AddJob(runtime, j) + scheduleSettingJob(s.cron, "LDAP sync", runtime, "@every 1m", job.NewLdapSyncJob()) } // Telegram-bot–dependent jobs: periodic stats report + callback-hash cleanup. @@ -399,16 +396,9 @@ func (s *Server) startTask(restartXray bool, loc *time.Location) { if (err == nil) && isTgbotenabled { runtime, err := s.settingService.GetTgbotRuntime() if err != nil { - logger.Warningf("Add NewStatsNotifyJob: failed to load runtime: %v; using default @daily", err) - runtime = "@daily" - } else if strings.TrimSpace(runtime) == "" { - logger.Warning("Add NewStatsNotifyJob runtime is empty, using default @daily") - runtime = "@daily" - } - logger.Infof("Tg notify enabled,run at %s", runtime) - if _, err = s.cron.AddJob(runtime, job.NewStatsNotifyJob()); err != nil { - logger.Warningf("Add NewStatsNotifyJob: failed to schedule runtime %q: %v", runtime, err) + logger.Warningf("Telegram notify: failed to load runtime: %v", err) } + scheduleSettingJob(s.cron, "Telegram notify", runtime, "@daily", job.NewStatsNotifyJob()) // check for Telegram bot callback query hash storage reset _, _ = s.cron.AddJob(cadenceCheckHash, job.NewCheckHashStorageJob()) @@ -419,18 +409,9 @@ func (s *Server) startTask(restartXray bool, loc *time.Location) { if (err == nil) && isDiscordEnabled { runtime, err := s.settingService.GetDiscordRunTime() if err != nil { - logger.Warningf("Add NewDiscordNotifyJob: failed to load runtime: %v; using default @daily", err) - runtime = "@daily" - } else if strings.TrimSpace(runtime) == "" { - logger.Warning("Add NewDiscordNotifyJob runtime is empty, using default @daily") - runtime = "@daily" - } - logger.Infof("Discord notify enabled, run at %s", runtime) - if entryID, err := s.cron.AddJob(runtime, job.NewDiscordNotifyJob(s.discordService)); err != nil { - logger.Warningf("Add NewDiscordNotifyJob: failed to schedule runtime %q: %v", runtime, err) - } else { - s.discordNotifyEntryID = entryID + logger.Warningf("Discord notify: failed to load runtime: %v", err) } + s.discordNotifyEntryID = scheduleSettingJob(s.cron, "Discord notify", runtime, "@daily", job.NewDiscordNotifyJob(s.discordService)) } // CPU monitor publishes cpu.high events; register it whenever any notifier @@ -710,16 +691,10 @@ func (s *Server) start(restartXray bool, startTgBot bool) (err error) { return } runtime, err := s.settingService.GetDiscordRunTime() - if err != nil || strings.TrimSpace(runtime) == "" { - runtime = "@daily" - } - entryID, err := s.cron.AddJob(runtime, job.NewDiscordNotifyJob(s.discordService)) if err != nil { - logger.Warningf("Reload Discord notify: failed to schedule runtime %q: %v", runtime, err) - } else { - s.discordNotifyEntryID = entryID - logger.Infof("Discord notify rescheduled, run at %s", runtime) + logger.Warningf("Discord notify: failed to load runtime: %v", err) } + s.discordNotifyEntryID = scheduleSettingJob(s.cron, "Discord notify", runtime, "@daily", job.NewDiscordNotifyJob(s.discordService)) if s.discordGateway != nil && !s.discordGateway.IsRunning() { _ = s.discordGateway.Start(s.ctx) @@ -859,3 +834,23 @@ func (s *Server) GetWSHub() any { func (s *Server) RestartXray() error { return s.xrayService.RestartXray(true) } + +// scheduleSettingJob schedules j on an admin-typed spec, running it on fallback +// when the spec is empty or a legacy value that no longer parses. +func scheduleSettingJob(c *cron.Cron, name, spec, fallback string, j cron.Job) cron.EntryID { + if strings.TrimSpace(spec) == "" { + logger.Warningf("%s: schedule is empty, using default %s", name, fallback) + spec = fallback + } + schedule, err := cronspec.Parse(spec) + if err != nil { + logger.Warningf("%s: %v; using default %s", name, err, fallback) + spec = fallback + if schedule, err = cronspec.Parse(fallback); err != nil { + logger.Errorf("%s: default schedule %q: %v", name, fallback, err) + return 0 + } + } + logger.Infof("%s scheduled, run at %s", name, spec) + return c.Schedule(schedule, j) +}