From fcbd9ae02ea8a41e94fe1a529d4ce59955c18c15 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Mon, 24 Aug 2026 13:23:07 +0800 Subject: [PATCH 1/2] =?UTF-8?q?fix=F0=9F=90=9B:=20an=20unconfigured=20obje?= =?UTF-8?q?ct=20store=20reports=20it=20instead=20of=20panicking?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each implementation keeps its provider client in an interface{} field that Setup assigns, so an unconfigured store holds nil - and asserting nil to the provider's client type panics: panic: interface conversion: interface {} is nil, not *oss.Client The upload endpoint reaches that path for any request naming a provider the deployment never configured. Three more things were wrong in the same files. OXS.Setup printed a failure and returned the store anyway, handing back exactly the broken object that panics. HuaWeiOBS.UpLoad printed the provider's error and returned nil, so a failed upload reported success. Both it and QiNiuKODO.UpLoad asserted the local path was a string without checking. The tests asked the reader to paste their own credentials, so they failed for everyone who did not. They now cover the guards and skip the part that needs a provider unless credentials are in the environment. --- common/file_store/errors.go | 20 ++++++++ common/file_store/file_store_test.go | 70 ++++++++++++++++++++++++++++ common/file_store/initialize.go | 32 +++++-------- common/file_store/kodo.go | 28 +++++++---- common/file_store/kodo_test.go | 23 --------- common/file_store/obs.go | 28 ++++++----- common/file_store/obs_test.go | 15 ------ common/file_store/oss.go | 6 ++- common/file_store/oss_test.go | 16 ------- 9 files changed, 143 insertions(+), 95 deletions(-) create mode 100644 common/file_store/errors.go create mode 100644 common/file_store/file_store_test.go delete mode 100644 common/file_store/kodo_test.go delete mode 100644 common/file_store/obs_test.go delete mode 100644 common/file_store/oss_test.go diff --git a/common/file_store/errors.go b/common/file_store/errors.go new file mode 100644 index 00000000..2464d2c3 --- /dev/null +++ b/common/file_store/errors.go @@ -0,0 +1,20 @@ +package file_store + +import "fmt" + +// ErrNotConfigured is returned by an object store that was never given +// credentials. The Client field of every implementation is an interface{} +// assigned in Setup, so an unconfigured store holds nil, and asserting nil to +// the provider's client type panics. The upload endpoint reaches this path +// whenever a request asks for a provider the deployment has not configured. +type ErrNotConfigured struct { + Driver DriverType +} + +func (e *ErrNotConfigured) Error() string { + return fmt.Sprintf("file store %s is not configured; set it under extend.fileStore", e.Driver) +} + +func notConfigured(driver DriverType) error { + return &ErrNotConfigured{Driver: driver} +} diff --git a/common/file_store/file_store_test.go b/common/file_store/file_store_test.go new file mode 100644 index 00000000..3ea07f89 --- /dev/null +++ b/common/file_store/file_store_test.go @@ -0,0 +1,70 @@ +package file_store + +import ( + "errors" + "os" + "testing" +) + +// The three implementations keep their provider client in an interface{} field +// that Setup assigns, so an unconfigured store holds nil. Asserting nil to the +// provider's type panics, and the upload endpoint reaches that path whenever a +// request names a provider the deployment never configured. +func TestUnconfiguredStoresReportItInsteadOfPanicking(t *testing.T) { + stores := map[DriverType]FileStoreType{ + AliYunOSS: new(ALiYunOSS), + HuaweiOBS: new(HuaWeiOBS), + QiNiuKodo: new(QiNiuKODO), + } + + for driver, store := range stores { + t.Run(string(driver), func(t *testing.T) { + err := store.UpLoad("img/x.png", "/tmp/x.png") + if err == nil { + t.Fatal("upload on an unconfigured store returned no error") + } + var notCfg *ErrNotConfigured + if !errors.As(err, ¬Cfg) { + t.Fatalf("want ErrNotConfigured, got %v", err) + } + if notCfg.Driver != driver { + t.Errorf("error names %s, want %s", notCfg.Driver, driver) + } + }) + } +} + +func TestUnconfiguredTokenReportsItToo(t *testing.T) { + if _, err := new(QiNiuKODO).GetTempToken(); err == nil { + t.Fatal("token from an unconfigured store returned no error") + } +} + +func TestSetupRejectsAnUnknownDriver(t *testing.T) { + if _, err := (&OXS{}).Setup("NoSuchCloud"); err == nil { + t.Fatal("unknown driver was accepted") + } +} + +// Setup reaching the provider needs credentials, so it runs only when they are +// supplied. Previously the test carried a comment telling the reader to paste +// their own, which meant it failed for everyone who did not. +func TestSetupWithRealCredentials(t *testing.T) { + endpoint := os.Getenv("GOADMIN_OSS_ENDPOINT") + if endpoint == "" { + t.Skip("set GOADMIN_OSS_ENDPOINT, GOADMIN_OSS_AK, GOADMIN_OSS_SK, GOADMIN_OSS_BUCKET to run") + } + oxs := OXS{ + Endpoint: endpoint, + AccessKeyID: os.Getenv("GOADMIN_OSS_AK"), + AccessKeySecret: os.Getenv("GOADMIN_OSS_SK"), + BucketName: os.Getenv("GOADMIN_OSS_BUCKET"), + } + store, err := oxs.Setup(AliYunOSS) + if err != nil { + t.Fatalf("setup: %v", err) + } + if store == nil { + t.Fatal("setup returned no store and no error") + } +} diff --git a/common/file_store/initialize.go b/common/file_store/initialize.go index fabce9fb..377972fa 100644 --- a/common/file_store/initialize.go +++ b/common/file_store/initialize.go @@ -14,32 +14,24 @@ type OXS struct { } // Setup 配置文件存储driver -func (e *OXS) Setup(driver DriverType, options ...ClientOption) FileStoreType { - fileStoreType := driver +// +// A failed Setup used to be printed and the store returned anyway, so the +// caller received one whose Client was nil - which panicked on first use. The +// error is returned instead. +func (e *OXS) Setup(driver DriverType, options ...ClientOption) (FileStoreType, error) { var fileStore FileStoreType - switch fileStoreType { + switch driver { case AliYunOSS: fileStore = new(ALiYunOSS) - err := fileStore.Setup(e.Endpoint, e.AccessKeyID, e.AccessKeySecret, e.BucketName) - if err != nil { - fmt.Println(err) - } - return fileStore case HuaweiOBS: fileStore = new(HuaWeiOBS) - err := fileStore.Setup(e.Endpoint, e.AccessKeyID, e.AccessKeySecret, e.BucketName) - if err != nil { - fmt.Println(err) - } - return fileStore case QiNiuKodo: fileStore = new(QiNiuKODO) - err := fileStore.Setup(e.Endpoint, e.AccessKeyID, e.AccessKeySecret, e.BucketName) - if err != nil { - fmt.Println(err) - } - return fileStore + default: + return nil, fmt.Errorf("unsupported file store driver %q", driver) } - - return nil + if err := fileStore.Setup(e.Endpoint, e.AccessKeyID, e.AccessKeySecret, e.BucketName, options...); err != nil { + return nil, fmt.Errorf("file store %s: %w", driver, err) + } + return fileStore, nil } diff --git a/common/file_store/kodo.go b/common/file_store/kodo.go index db896e27..1cad8d29 100644 --- a/common/file_store/kodo.go +++ b/common/file_store/kodo.go @@ -29,15 +29,20 @@ type QiNiuKODO struct { options []ClientOption } -func (e *QiNiuKODO) getToken() string { +func (e *QiNiuKODO) getToken() (string, error) { + mac, ok := e.Client.(*qbox.Mac) + if !ok { + return "", notConfigured(QiNiuKodo) + } putPolicy := storage.PutPolicy{ Scope: e.BucketName, } - if len(e.options) > 0 && e.options[0]["Expires"] != nil { - putPolicy.Expires = e.options[0]["Expires"].(uint64) + if len(e.options) > 0 { + if expires, ok := e.options[0]["Expires"].(uint64); ok { + putPolicy.Expires = expires + } } - upToken := putPolicy.UploadToken(e.Client.(*qbox.Mac)) - return upToken + return putPolicy.UploadToken(mac), nil } //Setup 装载 @@ -96,16 +101,21 @@ func (e *QiNiuKODO) UpLoad(yourObjectName string, localFile interface{}) error { "x:name": "github logo", }, } - err := formUploader.PutFile(context.Background(), &ret, e.getToken(), yourObjectName, localFile.(string), &putExtra) + token, err := e.getToken() if err != nil { - fmt.Println(err) return err } + source, ok := localFile.(string) + if !ok { + return fmt.Errorf("kodo upload wants a path, got %T", localFile) + } + if err := formUploader.PutFile(context.Background(), &ret, token, yourObjectName, source, &putExtra); err != nil { + return fmt.Errorf("kodo upload: %w", err) + } fmt.Println(ret.Key, ret.Hash) return nil } func (e *QiNiuKODO) GetTempToken() (string, error) { - token := e.getToken() - return token, nil + return e.getToken() } diff --git a/common/file_store/kodo_test.go b/common/file_store/kodo_test.go deleted file mode 100644 index a8767ae7..00000000 --- a/common/file_store/kodo_test.go +++ /dev/null @@ -1,23 +0,0 @@ -package file_store - -import ( - "testing" -) - -func TestKODOUpload(t *testing.T) { - e := OXS{"", "", "", ""} - var oxs = e.Setup(QiNiuKodo, map[string]interface{}{"Zone": "华东"}) - err := oxs.UpLoad("test.png", "./test.png") - if err != nil { - t.Error(err) - } - t.Log("ok") -} - -func TestKODOGetTempToken(t *testing.T) { - e := OXS{"", "", "", ""} - var oxs = e.Setup(QiNiuKodo, map[string]interface{}{"Zone": "华东"}) - token, _ := oxs.GetTempToken() - t.Log(token) - t.Log("ok") -} diff --git a/common/file_store/obs.go b/common/file_store/obs.go index e9377291..a7d2ad9e 100644 --- a/common/file_store/obs.go +++ b/common/file_store/obs.go @@ -26,24 +26,30 @@ func (e *HuaWeiOBS) Setup(endpoint, accessKeyID, accessKeySecret, BucketName str // UpLoad 文件上传 // yourObjectName 文件路径名称，与objectKey是同一概念，表示断点续传上传文件到OSS时需要指定包含文件后缀在内的完整路径，例如abc/efg/123.jpg func (e *HuaWeiOBS) UpLoad(yourObjectName string, localFile interface{}) error { + client, ok := e.Client.(*obs.ObsClient) + if !ok { + return notConfigured(HuaweiOBS) + } + source, ok := localFile.(string) + if !ok { + return fmt.Errorf("obs upload wants a path, got %T", localFile) + } + // 获取存储空间。 input := &obs.PutFileInput{} input.Bucket = e.BucketName input.Key = yourObjectName - input.SourceFile = localFile.(string) - output, err := e.Client.(*obs.ObsClient).PutFile(input) - - if err == nil { - fmt.Printf("RequestId:%s\n", output.RequestId) - fmt.Printf("ETag:%s, StorageClass:%s\n", output.ETag, output.StorageClass) - } else { + input.SourceFile = source + output, err := client.PutFile(input) + if err != nil { + // The error used to be printed and nil returned, so a failed upload + // reported success to the caller. if obsError, ok := err.(obs.ObsError); ok { - fmt.Println(obsError.Code) - fmt.Println(obsError.Message) - } else { - fmt.Println(err) + return fmt.Errorf("obs upload: %s: %s", obsError.Code, obsError.Message) } + return fmt.Errorf("obs upload: %w", err) } + log.Printf("obs upload ok, requestId=%s etag=%s", output.RequestId, output.ETag) return nil } diff --git a/common/file_store/obs_test.go b/common/file_store/obs_test.go deleted file mode 100644 index 09607508..00000000 --- a/common/file_store/obs_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package file_store - -import ( - "testing" -) - -func TestOBSUpload(t *testing.T) { - e := OXS{"", "", "", ""} - var oxs = e.Setup(HuaweiOBS) - err := oxs.UpLoad("test.png", "./test.png") - if err != nil { - t.Error(err) - } - t.Log("ok") -} diff --git a/common/file_store/oss.go b/common/file_store/oss.go index c35627b9..2a182eaf 100644 --- a/common/file_store/oss.go +++ b/common/file_store/oss.go @@ -26,8 +26,12 @@ func (e *ALiYunOSS) Setup(endpoint, accessKeyID, accessKeySecret, BucketName str // UpLoad 文件上传 func (e *ALiYunOSS) UpLoad(yourObjectName string, localFile interface{}) error { + client, ok := e.Client.(*oss.Client) + if !ok { + return notConfigured(AliYunOSS) + } // 获取存储空间。 - bucket, err := e.Client.(*oss.Client).Bucket(e.BucketName) + bucket, err := client.Bucket(e.BucketName) if err != nil { log.Println("Error:", err) return err diff --git a/common/file_store/oss_test.go b/common/file_store/oss_test.go deleted file mode 100644 index f04dd225..00000000 --- a/common/file_store/oss_test.go +++ /dev/null @@ -1,16 +0,0 @@ -package file_store - -import ( - "testing" -) - -func TestOSSUpload(t *testing.T) { - // 打括号内填写自己的测试信息即可 - e := OXS{} - var oxs = e.Setup(AliYunOSS) - err := oxs.UpLoad("test.png", "./test.png") - if err != nil { - t.Error(err) - } - t.Log("ok") -} From 04c6a081ae451bc001b0fc897ab6f1279d5ed1c1 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Mon, 24 Aug 2026 13:25:54 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix=F0=9F=90=9B:=20source=3D3=20uploaded=20?= =?UTF-8?q?to=20aliyun,=20and=20neither=20provider=20was=20ever=20configur?= =?UTF-8?q?ed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit thirdUpload dispatched on the source parameter and then built the same zero-value ALiYunOSS in both branches, so source=3 could not have reached qiniu even with credentials. Neither branch had credentials to use. OXS.Setup is the initialisation path and nothing in the repository called it, and no configuration field existed to fill. The store is now taken from extend.fileStore, and a provider that was not configured says so rather than producing the provider's own complaint about an empty bucket name. The two handlers passed errors.New("") to e.Error, discarding what actually went wrong; they now pass the error. --- app/other/apis/file.go | 39 ++++++++++++++++++++++++----------- app/other/apis/upload_test.go | 38 ++++++++++++++++++++++++++++++++++ config/extend.go | 33 +++++++++++++++++++++++++---- config/extend_test.go | 16 ++++++++++++++ config/settings.full.yml | 14 +++++++++++++ 5 files changed, 124 insertions(+), 16 deletions(-) create mode 100644 app/other/apis/upload_test.go create mode 100644 config/extend_test.go diff --git a/app/other/apis/file.go b/app/other/apis/file.go index 8cea78b4..fe8f56d2 100644 --- a/app/other/apis/file.go +++ b/app/other/apis/file.go @@ -14,6 +14,7 @@ import ( "github.com/google/uuid" "go-admin/common/file_store" + "go-admin/config" ) type FileResponse struct { @@ -95,7 +96,7 @@ func (e File) baseImg(c *gin.Context, fileResponse FileResponse, urlPrefix strin source, _ := c.GetPostForm("source") if err := thirdUpload(source, fileName, base64File); err != nil { - e.Error(200, errors.New(""), "上传第三方失败") + e.Error(200, err, "上传第三方失败") return fileResponse } @@ -126,7 +127,7 @@ func (e File) multipleFile(c *gin.Context, urlPrefix string) []FileResponse { fileType, _ := utils.GetType(multipartFileName) if err := thirdUpload(source, fileName, multipartFileName); err != nil { - e.Error(500, errors.New(""), "上传第三方失败") + e.Error(500, err, "上传第三方失败") continue } @@ -176,22 +177,36 @@ func (e File) buildFileResponse(filePath, urlPrefix, fileName, fileType string) } } +// thirdUpload copies the file that was already stored locally to the object +// store the request asked for. source "1", and anything unrecognised, keeps the +// local copy only. +// +// Both branches used to construct a zero-value ALiYunOSS and call UpLoad on it, +// which panicked - and the qiniu branch constructed the aliyun client, so +// source=3 never reached qiniu even in principle. func thirdUpload(source string, name string, path string) error { switch source { case "2": - return ossUpload("img/"+name, path) + return upload(file_store.AliYunOSS, config.ExtConfig.FileStore.AliYun, "img/"+name, path) case "3": - return qiniuUpload("img/"+name, path) + return upload(file_store.QiNiuKodo, config.ExtConfig.FileStore.QiNiu, "img/"+name, path) } return nil } -func ossUpload(name string, path string) error { - oss := file_store.ALiYunOSS{} - return oss.UpLoad(name, path) -} - -func qiniuUpload(name string, path string) error { - oss := file_store.ALiYunOSS{} - return oss.UpLoad(name, path) +func upload(driver file_store.DriverType, store config.ObjectStore, name, path string) error { + if !store.Configured() { + return fmt.Errorf("file store %s is not configured; set it under extend.fileStore", driver) + } + oxs := file_store.OXS{ + Endpoint: store.Endpoint, + AccessKeyID: store.AccessKeyID, + AccessKeySecret: store.AccessKeySecret, + BucketName: store.BucketName, + } + client, err := oxs.Setup(driver) + if err != nil { + return err + } + return client.UpLoad(name, path) } diff --git a/app/other/apis/upload_test.go b/app/other/apis/upload_test.go new file mode 100644 index 00000000..9831ff2d --- /dev/null +++ b/app/other/apis/upload_test.go @@ -0,0 +1,38 @@ +package apis + +import ( + "strings" + "testing" + + "go-admin/config" +) + +// Both branches used to construct a zero-value ALiYunOSS and call UpLoad on it, +// which panicked; the qiniu branch built the aliyun client, so source=3 could +// not have reached qiniu even with credentials. Unconfigured now reports which +// store is missing. +func TestThirdUploadReportsAnUnconfiguredStore(t *testing.T) { + previous := config.ExtConfig.FileStore + config.ExtConfig.FileStore = config.FileStore{} + t.Cleanup(func() { config.ExtConfig.FileStore = previous }) + + for source, want := range map[string]string{"2": "AliYunOSS", "3": "QiNiuKodo"} { + err := thirdUpload(source, "x.png", "/tmp/x.png") + if err == nil { + t.Errorf("source=%s: no error from an unconfigured store", source) + continue + } + if !strings.Contains(err.Error(), want) { + t.Errorf("source=%s: error names %q, want it to mention %s", source, err, want) + } + } +} + +// source 1 and anything unrecognised keep the local copy and do nothing else. +func TestThirdUploadIgnoresLocalAndUnknownSources(t *testing.T) { + for _, source := range []string{"", "1", "9"} { + if err := thirdUpload(source, "x.png", "/tmp/x.png"); err != nil { + t.Errorf("source=%q returned %v, want nil", source, err) + } + } +} diff --git a/config/extend.go b/config/extend.go index bd06e031..e8d6daa6 100644 --- a/config/extend.go +++ b/config/extend.go @@ -3,14 +3,39 @@ package config var ExtConfig Extend // Extend 扩展配置 -// extend: -// demo: -// name: demo-name +// +// extend: +// demo: +// name: demo-name +// // 使用方法： config.ExtConfig......即可！！ type Extend struct { - AMap AMap // 这里配置对应配置文件的结构即可 + AMap AMap // 这里配置对应配置文件的结构即可 + FileStore FileStore } type AMap struct { Key string } + +// FileStore 对象存储。上传接口的 source 参数决定走哪一家：2 是阿里云，3 是七牛。 +// 没有填的那一家在被请求时返回明确错误，而不是上传到别处或者崩溃。 +// +// common/file_store 里还实现了华为云 OBS，但上传接口没有对应的 source 取值， +// 所以这里也不为它提供配置。 +type FileStore struct { + AliYun ObjectStore + QiNiu ObjectStore +} + +type ObjectStore struct { + Endpoint string + AccessKeyID string + AccessKeySecret string + BucketName string +} + +// Configured reports whether enough was filled in to attempt a connection. +func (o ObjectStore) Configured() bool { + return o.Endpoint != "" && o.AccessKeyID != "" && o.AccessKeySecret != "" && o.BucketName != "" +} diff --git a/config/extend_test.go b/config/extend_test.go new file mode 100644 index 00000000..230174f4 --- /dev/null +++ b/config/extend_test.go @@ -0,0 +1,16 @@ +package config + +import "testing" + +func TestObjectStoreConfigured(t *testing.T) { + if (ObjectStore{}).Configured() { + t.Fatal("empty store reported as configured") + } + full := ObjectStore{Endpoint: "e", AccessKeyID: "a", AccessKeySecret: "s", BucketName: "b"} + if !full.Configured() { + t.Fatal("complete store reported as unconfigured") + } + if (ObjectStore{Endpoint: "e", AccessKeyID: "a"}).Configured() { + t.Fatal("partial store reported as configured") + } +} diff --git a/config/settings.full.yml b/config/settings.full.yml index bcca7e29..a1fdacce 100644 --- a/config/settings.full.yml +++ b/config/settings.full.yml @@ -56,3 +56,17 @@ settings: extend: # 扩展项使用说明 demo: name: data + # fileStore 对象存储。上传接口的 source 参数决定走哪一家： + # source=1 只存本地，source=2 阿里云 OSS，source=3 七牛 Kodo + # 没有填的那一家在被请求时会返回明确错误，不会静默存到别处。 + fileStore: + aliYun: + endpoint: oss-cn-hangzhou.aliyuncs.com + accessKeyId: "" + accessKeySecret: "" + bucketName: "" + qiNiu: + endpoint: "" + accessKeyId: "" + accessKeySecret: "" + bucketName: ""