diff --git a/app/geodata/download_test.go b/app/geodata/download_test.go index c7e225aa7..981ebab77 100644 --- a/app/geodata/download_test.go +++ b/app/geodata/download_test.go @@ -7,10 +7,13 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" commongeodata "github.com/xtls/xray-core/common/geodata" xlua "github.com/xtls/xray-core/common/lua" + "github.com/xtls/xray-core/common/serial" + "github.com/xtls/xray-core/core" "github.com/yuin/gopher-lua/parse" "google.golang.org/protobuf/proto" ) @@ -71,3 +74,41 @@ func TestDownloadOneLuaGuard(t *testing.T) { }) } } + +func TestProtobufConfigValidation(t *testing.T) { + server, err := core.New(&core.Config{}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = server.Close() }) + for _, tc := range []struct { + name string + config *Config + wantError string + }{ + {"minutely cron", &Config{Cron: "* * * * *"}, ""}, + {"minimum interval", &Config{Cron: "@every 1m"}, ""}, + {"short interval", &Config{Cron: "@every 59s"}, "interval must be at least 1 minute"}, + {"fractional interval", &Config{Cron: "@every 59.999s"}, "interval must be at least 1 minute"}, + {"invalid cron", &Config{Cron: "invalid"}, "invalid geodata cron"}, + {"dat extension", &Config{Cron: "@daily", Assets: []*Asset{{Url: "https://example.com/asset.dat", File: "asset.dat"}}}, ""}, + {"uppercase extension", &Config{Cron: "@daily", Assets: []*Asset{{Url: "https://example.com/asset.dat", File: "asset.DAT"}}}, ""}, + {"invalid extension", &Config{Cron: "@daily", Assets: []*Asset{{Url: "https://example.com/asset.dat", File: "asset.lua"}}}, "extension must be .dat"}, + {"missing extension", &Config{Cron: "@daily", Assets: []*Asset{{Url: "https://example.com/asset.dat", File: "asset"}}}, "extension must be .dat"}, + } { + t.Run(tc.name, func(t *testing.T) { + config, err := serial.ToTypedMessage(tc.config).GetInstance() + if err != nil { + t.Fatal(err) + } + _, err = core.CreateObject(server, config) + if tc.wantError == "" { + if err != nil { + t.Fatal(err) + } + } else if err == nil || !strings.Contains(err.Error(), tc.wantError) { + t.Fatalf("CreateObject() error = %v; want %q", err, tc.wantError) + } + }) + } +} diff --git a/app/geodata/geodata.go b/app/geodata/geodata.go index 5b0111a81..334020615 100644 --- a/app/geodata/geodata.go +++ b/app/geodata/geodata.go @@ -2,7 +2,10 @@ package geodata import ( "context" + "path/filepath" + "strings" "sync" + "time" "github.com/robfig/cron/v3" "github.com/xtls/xray-core/common" @@ -25,6 +28,18 @@ func New(ctx context.Context, config *Config) (*Instance, error) { if config.Cron == "" { return &Instance{}, nil } + for _, asset := range config.Assets { + if !strings.EqualFold(filepath.Ext(asset.GetFile()), ".dat") { + return nil, errors.New("geodata asset file extension must be .dat") + } + } + schedule, err := cron.ParseStandard(config.Cron) + if err != nil { + return nil, errors.New("invalid geodata cron").Base(err) + } + if interval, ok := schedule.(cron.ConstantDelaySchedule); ok && interval.Delay < time.Minute { + return nil, errors.New("geodata cron interval must be at least 1 minute") + } g := &Instance{ assets: config.Assets, @@ -44,9 +59,7 @@ func New(ctx context.Context, config *Config) (*Instance, error) { cron.WithChain(cron.SkipIfStillRunning(cron.DiscardLogger)), cron.WithLogger(cron.DiscardLogger), ) - if _, err := g.tasker.AddFunc(config.Cron, g.execute); err != nil { - return nil, errors.New("invalid geodata cron").Base(err) - } + g.tasker.Schedule(schedule, cron.FuncJob(g.execute)) errors.LogInfo(ctx, "scheduled geodata reload with cron: ", config.Cron) return g, nil diff --git a/infra/conf/geodata.go b/infra/conf/geodata.go index 42b0961bf..f79a37e23 100644 --- a/infra/conf/geodata.go +++ b/infra/conf/geodata.go @@ -4,7 +4,6 @@ import ( "net/url" "path/filepath" "strings" - "time" "github.com/robfig/cron/v3" "github.com/xtls/xray-core/app/geodata" @@ -45,17 +44,6 @@ func validateHTTPS(s string) error { return nil } -func validateGeodataCron(spec string) error { - schedule, err := cron.ParseStandard(spec) - if err != nil { - return err - } - if interval, ok := schedule.(cron.ConstantDelaySchedule); ok && interval.Delay < time.Minute { - return errors.New("interval must be at least 1 minute") - } - return nil -} - type GeodataConfig struct { Cron *string `json:"cron"` Outbound string `json:"outbound"` @@ -66,7 +54,7 @@ func (c *GeodataConfig) Build() (proto.Message, error) { config := &geodata.Config{} if c.Cron != nil { - if err := validateGeodataCron(*c.Cron); err != nil { + if _, err := cron.ParseStandard(*c.Cron); err != nil { return nil, errors.New("invalid geodata cron").Base(err) } config.Cron = *c.Cron diff --git a/infra/conf/geodata_test.go b/infra/conf/geodata_test.go index 97fe03087..4944d850a 100644 --- a/infra/conf/geodata_test.go +++ b/infra/conf/geodata_test.go @@ -106,42 +106,3 @@ func TestGeodataAssetConfigFileExtension(t *testing.T) { } } } - -func TestGeodataConfigCronFrequency(t *testing.T) { - for _, tc := range []struct { - cron string - wantError string - }{ - {"0 * * * *", ""}, - {"15 */2 * * *", ""}, - {"0 4 * * *", ""}, - {"*/60 * * * *", ""}, - {"0,0 * * * *", ""}, - {"CRON_TZ=Asia/Singapore 30 * * * *", ""}, - {"@hourly", ""}, - {"@daily", ""}, - {"@every 1m", ""}, - {"@every 1h", ""}, - {"@every 1h30m", ""}, - {"* * * * *", ""}, - {"*/30 * * * *", ""}, - {"0,59 4 * * *", ""}, - {"0-1 4 1 1 *", ""}, - {"@every 59m59s", ""}, - {"@every 1s", "interval must be at least 1 minute"}, - {"@every 59s", "interval must be at least 1 minute"}, - {"@every 59.999s", "interval must be at least 1 minute"}, - {"invalid", "invalid geodata cron"}, - } { - t.Run(tc.cron, func(t *testing.T) { - _, err := (&GeodataConfig{Cron: &tc.cron}).Build() - if tc.wantError == "" { - if err != nil { - t.Fatal(err) - } - } else if err == nil || !strings.Contains(err.Error(), tc.wantError) { - t.Fatalf("Build() error = %v; want %q", err, tc.wantError) - } - }) - } -}