Skip to content

feat(parity): real getHTML export, padCopy/padRemove hooks, public-status guard, listAuthors filter #284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SamTV12345 merged 1 commit into from
Jun 12, 2026
+198 −25
Merged

feat(parity): real getHTML export, padCopy/padRemove hooks, public-status guard, listAuthors filter #284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions lib/api/errors/httpErrors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ var PadNotFoundError = Error{
Error: 404,
}

// NotAGroupPadError mirrors the original Etherpad checkGroupPad guard: public
// status can only be read or changed on pads that belong to a group.
var NotAGroupPadError = Error{
Message: "You can only get/set the publicStatus of pads that belong to a group",
Error: 400,
}

var AuthorNotFoundError = Error{
Message: "Author not found",
Error: 404,
Expand Down
46 changes: 43 additions & 3 deletions lib/api/pad/copyMove.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"github.com/ether/etherpad-go/lib"
errors2 "github.com/ether/etherpad-go/lib/api/errors"
utils2 "github.com/ether/etherpad-go/lib/api/utils"
"github.com/ether/etherpad-go/lib/hooks"
db2 "github.com/ether/etherpad-go/lib/models/db"
padModel "github.com/ether/etherpad-go/lib/models/pad"
"github.com/gofiber/fiber/v3"
)

Expand Down Expand Up @@ -106,6 +108,28 @@ func copyPadRecords(initStore *lib.InitStore, sourceID string, destinationID str
return nil
}

// firePadCopy notifies plugins that a pad was copied, mirroring the original
// Etherpad padCopy hook which is fired with the source and destination pads.
func firePadCopy(initStore *lib.InitStore, srcPad *padModel.Pad, dstId string) {
dstPad, err := initStore.PadManager.GetPad(dstId, nil, nil)
if err != nil {
initStore.Logger.Errorf("padCopy hook: failed to load destination pad %s: %v", dstId, err)
return
}
initStore.Hooks.ExecuteHooks(hooks.PadCopyString, padModel.Copy{
SrcPad: srcPad,
DstPad: dstPad,
SrcId: srcPad.Id,
DstId: dstId,
})
}

// isGroupPad mirrors the original Etherpad checkGroupPad guard: only pads that
// belong to a group (their id contains a '$') expose a public status.
func isGroupPad(padId string) bool {
return strings.Contains(padId, "$")
}

// CopyPad godoc
// @Summary Copy a pad
// @Description Copies a pad including its full revision and chat history to a new pad. Fails if the destination exists unless force is set.
Expand All @@ -130,7 +154,8 @@ func CopyPad(initStore *lib.InitStore) fiber.Handler {
}

// Verify source pad exists
if _, err := utils2.GetPadSafe(padId, true, nil, nil, initStore.PadManager); err != nil {
srcPad, err := utils2.GetPadSafe(padId, true, nil, nil, initStore.PadManager)
if err != nil {
return c.Status(404).JSON(errors2.PadNotFoundError)
}

Expand All @@ -143,6 +168,8 @@ func CopyPad(initStore *lib.InitStore) fiber.Handler {
return c.Status(500).JSON(errors2.InternalServerError)
}

firePadCopy(initStore, srcPad, request.DestinationID)

return c.JSON(PadIDResponse{
PadID: request.DestinationID,
})
Expand Down Expand Up @@ -195,6 +222,8 @@ func CopyPadWithoutHistory(initStore *lib.InitStore) fiber.Handler {
return c.Status(500).JSON(errors2.InternalServerError)
}

firePadCopy(initStore, sourcePad, request.DestinationID)

return c.JSON(PadIDResponse{
PadID: request.DestinationID,
})
Expand Down Expand Up @@ -225,7 +254,8 @@ func MovePad(initStore *lib.InitStore) fiber.Handler {
}

// Verify source pad exists
if _, err := utils2.GetPadSafe(padId, true, nil, nil, initStore.PadManager); err != nil {
srcPad, err := utils2.GetPadSafe(padId, true, nil, nil, initStore.PadManager)
if err != nil {
return c.Status(404).JSON(errors2.PadNotFoundError)
}

Expand All @@ -238,7 +268,9 @@ func MovePad(initStore *lib.InitStore) fiber.Handler {
return c.Status(500).JSON(errors2.InternalServerError)
}

// Remove the source pad after a successful copy
firePadCopy(initStore, srcPad, request.DestinationID)

// Remove the source pad after a successful copy (fires padRemove)
if err := initStore.PadManager.RemovePad(padId); err != nil {
return c.Status(500).JSON(errors2.InternalServerError)
}
Expand All @@ -264,6 +296,10 @@ func GetPublicStatus(initStore *lib.InitStore) fiber.Handler {
return func(c fiber.Ctx) error {
padId := c.Params("padId")

if !isGroupPad(padId) {
return c.Status(400).JSON(errors2.NotAGroupPadError)
}

// Get the pad
pad, err := utils2.GetPadSafe(padId, true, nil, nil, initStore.PadManager)
if err != nil {
Expand Down Expand Up @@ -298,6 +334,10 @@ func SetPublicStatus(initStore *lib.InitStore) fiber.Handler {
return c.Status(400).JSON(errors2.InvalidRequestError)
}

if !isGroupPad(padId) {
return c.Status(400).JSON(errors2.NotAGroupPadError)
}

// Get the pad
pad, err := utils2.GetPadSafe(padId, true, nil, nil, initStore.PadManager)
if err != nil {
Expand Down
30 changes: 15 additions & 15 deletions lib/api/pad/operations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
utils2 "github.com/ether/etherpad-go/lib/api/utils"
"github.com/ether/etherpad-go/lib/apool"
"github.com/ether/etherpad-go/lib/changeset"
io2 "github.com/ether/etherpad-go/lib/io"
padModel "github.com/ether/etherpad-go/lib/models/pad"
"github.com/ether/etherpad-go/lib/utils"
"github.com/gofiber/fiber/v3"
)
Expand Down Expand Up @@ -253,10 +255,13 @@ func ListAuthorsOfPad(initStore *lib.InitStore) fiber.Handler {
return c.Status(404).JSON(errors2.PadNotFoundError)
}

// Get all authors from the pool
// Get all authors from the pool. SystemAuthorId is the synthetic author
// used for unattributed inserts (HTTP API writes without an authorId,
// server-side imports); it is a changeset-bookkeeping detail, not a real
// contributor, so it must not surface through this public API.
authorIDs := make([]string, 0)
pad.Pool.EachAttrib(func(attr apool.Attribute) {
if attr.Key == "author" && attr.Value != "" {
if attr.Key == "author" && attr.Value != "" && attr.Value != padModel.SystemAuthorId {
// Check if not already in list
found := false
for _, id := range authorIDs {
Expand Down Expand Up @@ -432,21 +437,16 @@ func GetHTML(initStore *lib.InitStore) fiber.Handler {
rev = revNum
}

// Get the HTML (using exporter if available)
var text string
if rev != nil {
atext := pad.GetInternalRevisionAText(*rev)
if atext == nil {
return c.Status(500).JSON(errors2.InternalApiError)
}
text = atext.Text
} else {
text = pad.Text()
// Render the full HTML document through the real exporter so that
// formatting (bold/italic/underline, links, lists, headings, line
// markers) is preserved. Mirrors the original Etherpad getHTML, which
// delegates to exportHtml.getPadHTMLDocument.
exporter := io2.NewExportHtml(initStore.PadManager, initStore.AuthorManager, initStore.Hooks)
html, err := exporter.GetPadHTMLDocument(padId, rev, nil)
if err != nil {
return c.Status(500).JSON(errors2.InternalApiError)
}

// Simple HTML conversion (basic)
html := "<html><body>" + strings.ReplaceAll(text, "\n", "<br>") + "</body></html>"

return c.JSON(fiber.Map{
"html": html,
})
Expand Down
2 changes: 2 additions & 0 deletions lib/hooks/HookConstants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ const PreAuthzFailureString = "preAuthzFailure"
const PadLoadString = "padLoad"
const PadCreateString = "padCreate"
const PadUpdateString = "padUpdate"
const PadCopyString = "padCopy"
const PadRemoveString = "padRemove"
16 changes: 16 additions & 0 deletions lib/models/pad/PadDefaultContent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,19 @@ type Create struct {
PadId string
AuthorId string
}

// Copy is the context passed to the padCopy hook after a pad is copied to a new
// destination (copyPad, copyPadWithoutHistory, movePad). Mirrors the original
// Etherpad context, which exposes the source and destination pads.
type Copy struct {
SrcPad *Pad
DstPad *Pad
SrcId string
DstId string
}

// Remove is the context passed to the padRemove hook when a pad is deleted.
type Remove struct {
Pad *Pad
PadId string
}
10 changes: 10 additions & 0 deletions lib/pad/padManager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,22 @@ func (m *Manager) SanitizePadId(padID string) (*string, error) {
}

func (m *Manager) RemovePad(padID string) error {
// Capture the loaded pad (if any) before deletion so the padRemove hook can
// hand listeners the pad context, mirroring the original Etherpad which
// fires padRemove from Pad.remove() with `this`.
removedPad := m.globalPadCache.GetPad(padID)

if err := m.store.RemovePad(padID); err != nil {
return err
}
m.globalPadCache.DeletePad(padID)
m.padList.RemovePad(padID)

m.hook.ExecuteHooks(hooks.PadRemoveString, pad.Remove{
Pad: removedPad,
PadId: padID,
})
Comment on lines +131 to +145

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Padremove pad can be nil 🐞 Bug ☼ Reliability

Manager.RemovePad only captures the pad from the in-memory cache and passes it to the new padRemove
hook, so ctx.Pad will be nil whenever the pad wasn't already loaded. This can happen on
force-overwrite copy destinations (removed without loading), causing plugins to panic on nil
dereference or lose access to removed pad state.
Agent Prompt 
### Issue description
`Manager.RemovePad()` fires the new `padRemove` hook with a `Pad` value taken only from `globalPadCache`. If the pad was never loaded (common for force-overwrite copy destinations), the hook receives `ctx.Pad == nil`, which can break plugin expectations and can crash the process if a hook callback dereferences it.

### Issue Context
The PR intent says the hook should capture the loaded pad before deletion (mirroring upstream). Today, `prepareCopyDestination()` can call `RemovePad(destinationID)` without ever loading that pad into the cache.

### Fix Focus Areas
- lib/pad/padManager.go[130-146]
- lib/api/pad/copyMove.go[33-44]

### Suggested fix approach
- In `Manager.RemovePad`, if `removedPad == nil`, check existence (`m.store.DoesPadExist(padID)`), and if it exists, load it (`m.GetPad(padID, nil, nil)`) *before* deleting so the hook gets a non-nil pad context.
- Alternatively (or additionally), in `prepareCopyDestination` (force path), load the destination pad via `GetPadSafe(destinationID, true, nil, nil, initStore.PadManager)` before calling `RemovePad`.
- Ensure you do not accidentally create a new pad when deleting a non-existent pad (so existence check must gate any load).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


return nil
}

Expand Down
Loading
Loading