fix: CORS + rate-limit + readonly pad IDs on /export/markdown#183
Open
JohnMcLear wants to merge 3 commits intomasterfrom
Open
fix: CORS + rate-limit + readonly pad IDs on /export/markdown#183JohnMcLear wants to merge 3 commits intomasterfrom
JohnMcLear wants to merge 3 commits intomasterfrom
Conversation
Mirrors the Access-Control-Allow-Origin header that Etherpad core sets on its native /export/* routes (see etherpad-lite's src/node/hooks/express/importexport.ts). Without this, browsers block repeated fetches of /p/:padId/export/markdown with a CORS error — which integrators hit when rate-limited exports get retried or when the export is initiated from a different origin than the pad. Closes #139 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Expands the previous CORS-only change so that /p/:padId/export/markdown
and /p/:padId/:revNum/export/markdown now match the full set of
protections that Etherpad core applies to its native /export/:type
routes (src/node/hooks/express/importexport.ts):
- Rate limit both routes with the core `importExportRateLimiting`
settings, using the shared `express-rate-limit` module. Without this,
a caller can bypass export throttling by switching from pdf/doc/txt/
html/etc. to markdown.
- Resolve readonly pad IDs (`r.*`) through ReadOnlyManager before
loading the pad. Previously padManager.getPad('r.xxx') would create
an empty pad under the readonly ID and the export returned nothing.
`express-rate-limit` is already shipped by Etherpad core; declared here
as a peerDependency so the requirement is explicit.
Refs #139 (CORS), #142 (prior attempt that bundled rate limit +
readonly fix + CORS — this commit brings its ideas into the current
TS-based codebase).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JohnMcLear
changed the title
ReadOnlyManager uses `export default { ... }`, so under CommonJS require()
the methods live on `.default` — `readOnlyManager.isReadOnlyId` was
undefined and `/export/markdown` threw 500. Unwrap explicitly so the
readonly-pad path actually resolves.
Caught by backend test "Import and Export markdown returns ok" on
the new resolvePadId call path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings the
/export/markdownroutes in line with the protections that Etherpad core applies to its native/export/:typeroutes (seeimportexport.ts):Access-Control-Allow-Origin: *) on both export routes — fixes the error reported in importExportRateLimiting setting ignored, CORS header ‘Access-Control-Allow-Origin’ missing #139 (browsers block repeated or cross-origin fetches).settings.importExportRateLimiting+ the sharedexpress-rate-limitmodule. Without this, callers could sidestep export throttling by switching format to markdown. Matches the exact middleware pattern used in core.r.*) are now resolved throughReadOnlyManagerbeforepadManager.getPadis called. PreviouslygetPad('r.xxx')created an empty pad under the readonly ID and the export returned nothing.Supersedes / incorporates the ideas from #142 (which did the same three things, but against the pre-TypeScript codebase and is therefore stale).
Files
express.ts— CORS header, rate-limit middleware for both route variantsexportMarkdown.ts— readonly pad ID resolution ingetPadMarkdownDocumentpackage.json— addsexpress-rate-limitas a peerDependency (core already ships it, this just documents the requirement)Test plan
Closes #139, supersedes #142.