From 610a948dcc3c1996d5d7fe038b5941b69baa2fc1 Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Thu, 20 Jul 2023 10:40:03 +0300 Subject: [PATCH] added Response.Committed checks --- apis/admin.go | 32 +++++++++++++++++++++++++++++++ apis/collection.go | 24 +++++++++++++++++++++++ apis/file.go | 12 +++++++++--- apis/realtime.go | 18 ++++++------------ apis/realtime_test.go | 1 - apis/record_auth.go | 40 +++++++++++++++++++++++++++++++++------ apis/record_crud.go | 20 ++++++++++++++++++++ apis/record_helpers.go | 4 ++++ apis/settings.go | 8 ++++++++ mails/admin.go | 15 +++++---------- mails/record.go | 43 +++++++++++++++--------------------------- 11 files changed, 157 insertions(+), 60 deletions(-) diff --git a/apis/admin.go b/apis/admin.go index d3ba5cd6..f2640d0e 100644 --- a/apis/admin.go +++ b/apis/admin.go @@ -51,6 +51,10 @@ func (api *adminApi) authResponse(c echo.Context, admin *models.Admin, finalizer event.Token = token return api.app.OnAdminAuthRequest().Trigger(event, func(e *core.AdminAuthEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(200, map[string]any{ "token": e.Token, "admin": e.Admin, @@ -132,6 +136,10 @@ func (api *adminApi) requestPasswordReset(c echo.Context) error { }) return api.app.OnAdminAfterRequestPasswordResetRequest().Trigger(event, func(e *core.AdminRequestPasswordResetEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) @@ -166,6 +174,10 @@ func (api *adminApi) confirmPasswordReset(c echo.Context) error { } return api.app.OnAdminAfterConfirmPasswordResetRequest().Trigger(event, func(e *core.AdminConfirmPasswordResetEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) @@ -196,6 +208,10 @@ func (api *adminApi) list(c echo.Context) error { event.Result = result return api.app.OnAdminsListRequest().Trigger(event, func(e *core.AdminsListEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.Result) }) } @@ -216,6 +232,10 @@ func (api *adminApi) view(c echo.Context) error { event.Admin = admin return api.app.OnAdminViewRequest().Trigger(event, func(e *core.AdminViewEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.Admin) }) } @@ -245,6 +265,10 @@ func (api *adminApi) create(c echo.Context) error { } return api.app.OnAdminAfterCreateRequest().Trigger(event, func(e *core.AdminCreateEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.Admin) }) }) @@ -287,6 +311,10 @@ func (api *adminApi) update(c echo.Context) error { } return api.app.OnAdminAfterUpdateRequest().Trigger(event, func(e *core.AdminUpdateEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.Admin) }) }) @@ -317,6 +345,10 @@ func (api *adminApi) delete(c echo.Context) error { } return api.app.OnAdminAfterDeleteRequest().Trigger(event, func(e *core.AdminDeleteEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) diff --git a/apis/collection.go b/apis/collection.go index 6e7d4179..05fc154b 100644 --- a/apis/collection.go +++ b/apis/collection.go @@ -48,6 +48,10 @@ func (api *collectionApi) list(c echo.Context) error { event.Result = result return api.app.OnCollectionsListRequest().Trigger(event, func(e *core.CollectionsListEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.Result) }) } @@ -63,6 +67,10 @@ func (api *collectionApi) view(c echo.Context) error { event.Collection = collection return api.app.OnCollectionViewRequest().Trigger(event, func(e *core.CollectionViewEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.Collection) }) } @@ -92,6 +100,10 @@ func (api *collectionApi) create(c echo.Context) error { } return api.app.OnCollectionAfterCreateRequest().Trigger(event, func(e *core.CollectionCreateEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.Collection) }) }) @@ -127,6 +139,10 @@ func (api *collectionApi) update(c echo.Context) error { } return api.app.OnCollectionAfterUpdateRequest().Trigger(event, func(e *core.CollectionUpdateEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.Collection) }) }) @@ -150,6 +166,10 @@ func (api *collectionApi) delete(c echo.Context) error { } return api.app.OnCollectionAfterDeleteRequest().Trigger(event, func(e *core.CollectionDeleteEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) @@ -178,6 +198,10 @@ func (api *collectionApi) bulkImport(c echo.Context) error { } return api.app.OnCollectionsAfterImportRequest().Trigger(event, func(e *core.CollectionsImportEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) diff --git a/apis/file.go b/apis/file.go index 25dcf2ef..873e1354 100644 --- a/apis/file.go +++ b/apis/file.go @@ -51,6 +51,10 @@ func (api *fileApi) fileToken(c echo.Context) error { } return api.app.OnFileAfterTokenRequest().Trigger(event, func(e *core.FileTokenEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, map[string]string{ "token": e.Token, }) @@ -168,9 +172,11 @@ func (api *fileApi) download(c echo.Context) error { c.Response().Header().Del("X-Frame-Options") return api.app.OnFileDownloadRequest().Trigger(event, func(e *core.FileDownloadEvent) error { - res := e.HttpContext.Response() - req := e.HttpContext.Request() - if err := fs.Serve(res, req, e.ServedPath, e.ServedName); err != nil { + if e.HttpContext.Response().Committed { + return nil + } + + if err := fs.Serve(e.HttpContext.Response(), e.HttpContext.Request(), e.ServedPath, e.ServedName); err != nil { return NewNotFoundError("", err) } diff --git a/apis/realtime.go b/apis/realtime.go index 6d9c86d5..7d4a38d1 100644 --- a/apis/realtime.go +++ b/apis/realtime.go @@ -92,7 +92,7 @@ func (api *realtimeApi) connect(c echo.Context) error { fmt.Fprint(w, "event:"+e.Message.Name+"\n") fmt.Fprint(w, "data:"+e.Message.Data+"\n\n") w.Flush() - return nil + return api.app.OnRealtimeAfterMessageSend().Trigger(e) }) if connectMsgErr != nil { if api.app.IsDebug() { @@ -100,9 +100,6 @@ func (api *realtimeApi) connect(c echo.Context) error { } return nil } - if err := api.app.OnRealtimeAfterMessageSend().Trigger(connectMsgEvent); err != nil && api.app.IsDebug() { - log.Println("OnRealtimeAfterMessageSend PB_CONNECT error:", err) - } // start an idle timer to keep track of inactive/forgotten connections idleDuration := 5 * time.Minute @@ -133,7 +130,7 @@ func (api *realtimeApi) connect(c echo.Context) error { fmt.Fprint(w, "event:"+e.Message.Name+"\n") fmt.Fprint(w, "data:"+e.Message.Data+"\n\n") w.Flush() - return nil + return api.app.OnRealtimeAfterMessageSend().Trigger(msgEvent) }) if msgErr != nil { if api.app.IsDebug() { @@ -142,13 +139,6 @@ func (api *realtimeApi) connect(c echo.Context) error { return nil } - if err := api.app.OnRealtimeAfterMessageSend().Trigger(msgEvent); err != nil { - if api.app.IsDebug() { - log.Println("Realtime connection closed (OnRealtimeAfterMessageSend error):", client.Id(), err) - } - return nil - } - idleTimer.Stop() idleTimer.Reset(idleDuration) case <-c.Request().Context().Done(): @@ -206,6 +196,10 @@ func (api *realtimeApi) setSubscriptions(c echo.Context) error { e.Client.Subscribe(e.Subscriptions...) return api.app.OnRealtimeAfterSubscribeRequest().Trigger(event, func(e *core.RealtimeSubscribeEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) diff --git a/apis/realtime_test.go b/apis/realtime_test.go index b715dcb5..3a9d09e7 100644 --- a/apis/realtime_test.go +++ b/apis/realtime_test.go @@ -72,7 +72,6 @@ func TestRealtimeConnect(t *testing.T) { ExpectedEvents: map[string]int{ "OnRealtimeConnectRequest": 1, "OnRealtimeBeforeMessageSend": 1, - "OnRealtimeAfterMessageSend": 1, "OnRealtimeDisconnectRequest": 1, }, BeforeTestFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { diff --git a/apis/record_auth.go b/apis/record_auth.go index a1b75338..e2ce0e6e 100644 --- a/apis/record_auth.go +++ b/apis/record_auth.go @@ -66,8 +66,8 @@ func (api *recordAuthApi) authRefresh(c echo.Context) error { event.Record = record return api.app.OnRecordBeforeAuthRefreshRequest().Trigger(event, func(e *core.RecordAuthRefreshEvent) error { - return RecordAuthResponse(api.app, e.HttpContext, e.Record, nil, func(t string) error { - return api.app.OnRecordAfterAuthRefreshRequest().Trigger(event) + return api.app.OnRecordAfterAuthRefreshRequest().Trigger(event, func(e *core.RecordAuthRefreshEvent) error { + return RecordAuthResponse(api.app, e.HttpContext, e.Record, nil) }) }) } @@ -250,8 +250,8 @@ func (api *recordAuthApi) authWithOAuth2(c echo.Context) error { IsNew: event.IsNewRecord, } - return RecordAuthResponse(api.app, e.HttpContext, e.Record, meta, func(t string) error { - return api.app.OnRecordAfterAuthWithOAuth2Request().Trigger(event) + return api.app.OnRecordAfterAuthWithOAuth2Request().Trigger(event, func(e *core.RecordAuthWithOAuth2Event) error { + return RecordAuthResponse(api.app, e.HttpContext, e.Record, meta) }) }) } @@ -286,8 +286,8 @@ func (api *recordAuthApi) authWithPassword(c echo.Context) error { return NewBadRequestError("Failed to authenticate.", err) } - return RecordAuthResponse(api.app, e.HttpContext, e.Record, nil, func(t string) error { - return api.app.OnRecordAfterAuthWithPasswordRequest().Trigger(event) + return api.app.OnRecordAfterAuthWithPasswordRequest().Trigger(event, func(e *core.RecordAuthWithPasswordEvent) error { + return RecordAuthResponse(api.app, e.HttpContext, e.Record, nil) }) }) } @@ -333,6 +333,10 @@ func (api *recordAuthApi) requestPasswordReset(c echo.Context) error { }) return api.app.OnRecordAfterRequestPasswordResetRequest().Trigger(event, func(e *core.RecordRequestPasswordResetEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) @@ -373,6 +377,10 @@ func (api *recordAuthApi) confirmPasswordReset(c echo.Context) error { } return api.app.OnRecordAfterConfirmPasswordResetRequest().Trigger(event, func(e *core.RecordConfirmPasswordResetEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) @@ -414,6 +422,10 @@ func (api *recordAuthApi) requestVerification(c echo.Context) error { }) return api.app.OnRecordAfterRequestVerificationRequest().Trigger(event, func(e *core.RecordRequestVerificationEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) @@ -454,6 +466,10 @@ func (api *recordAuthApi) confirmVerification(c echo.Context) error { } return api.app.OnRecordAfterConfirmVerificationRequest().Trigger(event, func(e *core.RecordConfirmVerificationEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) @@ -492,6 +508,10 @@ func (api *recordAuthApi) requestEmailChange(c echo.Context) error { } return api.app.OnRecordAfterRequestEmailChangeRequest().Trigger(event, func(e *core.RecordRequestEmailChangeEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) @@ -524,6 +544,10 @@ func (api *recordAuthApi) confirmEmailChange(c echo.Context) error { } return api.app.OnRecordAfterConfirmEmailChangeRequest().Trigger(event, func(e *core.RecordConfirmEmailChangeEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) @@ -599,6 +623,10 @@ func (api *recordAuthApi) unlinkExternalAuth(c echo.Context) error { } return api.app.OnRecordAfterUnlinkExternalAuthRequest().Trigger(event, func(e *core.RecordUnlinkExternalAuthEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) diff --git a/apis/record_crud.go b/apis/record_crud.go index 424839e5..d35fef8b 100644 --- a/apis/record_crud.go +++ b/apis/record_crud.go @@ -91,6 +91,10 @@ func (api *recordApi) list(c echo.Context) error { event.Result = result return api.app.OnRecordsListRequest().Trigger(event, func(e *core.RecordsListEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + if err := EnrichRecords(e.HttpContext, api.app.Dao(), e.Records); err != nil && api.app.IsDebug() { log.Println(err) } @@ -141,6 +145,10 @@ func (api *recordApi) view(c echo.Context) error { event.Record = record return api.app.OnRecordViewRequest().Trigger(event, func(e *core.RecordViewEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + if err := EnrichRecord(e.HttpContext, api.app.Dao(), e.Record); err != nil && api.app.IsDebug() { log.Println(err) } @@ -239,6 +247,10 @@ func (api *recordApi) create(c echo.Context) error { } return api.app.OnRecordAfterCreateRequest().Trigger(event, func(e *core.RecordCreateEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.Record) }) }) @@ -322,6 +334,10 @@ func (api *recordApi) update(c echo.Context) error { } return api.app.OnRecordAfterUpdateRequest().Trigger(event, func(e *core.RecordUpdateEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.Record) }) }) @@ -377,6 +393,10 @@ func (api *recordApi) delete(c echo.Context) error { } return api.app.OnRecordAfterDeleteRequest().Trigger(event, func(e *core.RecordDeleteEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.NoContent(http.StatusNoContent) }) }) diff --git a/apis/record_helpers.go b/apis/record_helpers.go index 33b28f25..d39fc733 100644 --- a/apis/record_helpers.go +++ b/apis/record_helpers.go @@ -85,6 +85,10 @@ func RecordAuthResponse( event.Meta = meta return app.OnRecordAuthRequest().Trigger(event, func(e *core.RecordAuthEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + // allow always returning the email address of the authenticated account e.Record.IgnoreEmailVisibility(true) diff --git a/apis/settings.go b/apis/settings.go index 8aa9d681..e5e96ddf 100644 --- a/apis/settings.go +++ b/apis/settings.go @@ -37,6 +37,10 @@ func (api *settingsApi) list(c echo.Context) error { event.RedactedSettings = settings return api.app.OnSettingsListRequest().Trigger(event, func(e *core.SettingsListEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + return e.HttpContext.JSON(http.StatusOK, e.RedactedSettings) }) } @@ -64,6 +68,10 @@ func (api *settingsApi) set(c echo.Context) error { } return api.app.OnSettingsAfterUpdateRequest().Trigger(event, func(e *core.SettingsUpdateEvent) error { + if e.HttpContext.Response().Committed { + return nil + } + redactedSettings, err := api.app.Settings().RedactClone() if err != nil { return NewBadRequestError("", err) diff --git a/mails/admin.go b/mails/admin.go index 4c5e6cc5..e4f484a2 100644 --- a/mails/admin.go +++ b/mails/admin.go @@ -2,7 +2,6 @@ package mails import ( "fmt" - "log" "net/mail" "github.com/pocketbase/pocketbase/core" @@ -67,15 +66,11 @@ func SendAdminPasswordReset(app core.App, admin *models.Admin) error { event.Admin = admin event.Meta = map[string]any{"token": token} - sendErr := app.OnMailerBeforeAdminResetPasswordSend().Trigger(event, func(e *core.MailerAdminEvent) error { - return e.MailClient.Send(e.Message) - }) - - if sendErr == nil { - if err := app.OnMailerAfterAdminResetPasswordSend().Trigger(event); err != nil && app.IsDebug() { - log.Println(err) + return app.OnMailerBeforeAdminResetPasswordSend().Trigger(event, func(e *core.MailerAdminEvent) error { + if err := e.MailClient.Send(e.Message); err != nil { + return err } - } - return sendErr + return app.OnMailerAfterAdminResetPasswordSend().Trigger(e) + }) } diff --git a/mails/record.go b/mails/record.go index e848bd8b..fc2a93da 100644 --- a/mails/record.go +++ b/mails/record.go @@ -2,7 +2,6 @@ package mails import ( "html/template" - "log" "net/mail" "github.com/pocketbase/pocketbase/core" @@ -44,17 +43,13 @@ func SendRecordPasswordReset(app core.App, authRecord *models.Record) error { event.Record = authRecord event.Meta = map[string]any{"token": token} - sendErr := app.OnMailerBeforeRecordResetPasswordSend().Trigger(event, func(e *core.MailerRecordEvent) error { - return e.MailClient.Send(e.Message) - }) - - if sendErr == nil { - if err := app.OnMailerAfterRecordResetPasswordSend().Trigger(event); err != nil && app.IsDebug() { - log.Println(err) + return app.OnMailerBeforeRecordResetPasswordSend().Trigger(event, func(e *core.MailerRecordEvent) error { + if err := e.MailClient.Send(e.Message); err != nil { + return err } - } - return sendErr + return app.OnMailerAfterRecordResetPasswordSend().Trigger(e) + }) } // SendRecordVerification sends a verification request email to the specified user. @@ -88,17 +83,13 @@ func SendRecordVerification(app core.App, authRecord *models.Record) error { event.Record = authRecord event.Meta = map[string]any{"token": token} - sendErr := app.OnMailerBeforeRecordVerificationSend().Trigger(event, func(e *core.MailerRecordEvent) error { - return e.MailClient.Send(e.Message) - }) - - if sendErr == nil { - if err := app.OnMailerAfterRecordVerificationSend().Trigger(event); err != nil && app.IsDebug() { - log.Println(err) + return app.OnMailerBeforeRecordVerificationSend().Trigger(event, func(e *core.MailerRecordEvent) error { + if err := e.MailClient.Send(e.Message); err != nil { + return err } - } - return sendErr + return app.OnMailerAfterRecordVerificationSend().Trigger(e) + }) } // SendUserChangeEmail sends a change email confirmation email to the specified user. @@ -135,17 +126,13 @@ func SendRecordChangeEmail(app core.App, record *models.Record, newEmail string) "newEmail": newEmail, } - sendErr := app.OnMailerBeforeRecordChangeEmailSend().Trigger(event, func(e *core.MailerRecordEvent) error { - return e.MailClient.Send(e.Message) - }) - - if sendErr == nil { - if err := app.OnMailerAfterRecordChangeEmailSend().Trigger(event); err != nil && app.IsDebug() { - log.Println(err) + return app.OnMailerBeforeRecordChangeEmailSend().Trigger(event, func(e *core.MailerRecordEvent) error { + if err := e.MailClient.Send(e.Message); err != nil { + return err } - } - return sendErr + return app.OnMailerAfterRecordChangeEmailSend().Trigger(e) + }) } func resolveEmailTemplate(