fix: strip target and rel from <a> tags when sending to Basecamp API#151
Open
xela92 wants to merge 1 commit intoneedmore:mainfrom
Open
fix: strip target and rel from <a> tags when sending to Basecamp API#151xela92 wants to merge 1 commit intoneedmore:mainfrom
xela92 wants to merge 1 commit intoneedmore:mainfrom
Conversation
The Basecamp rich text spec only allows href on <a> tags, but the API itself returns links with target="_blank" rel="noreferrer" attributes. This makes round-trips fail: content fetched via `document view --json` that contains hyperlinks would be rejected by ValidateBasecampHTML when passed back to a create/edit request. Adds NormalizeRichText(), which strips target and rel before validation, so API-returned content can be safely round-tripped without relaxing the validator. stripUnsupportedTags() is also updated to handle these attributes so the markdown pipeline benefits from the same normalisation. Adds tests for both ValidateBasecampHTML (still rejects target/rel) and NormalizeRichText (strips them and accepts the result).
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.
Problem
ValidateBasecampHTMLcorrectly enforces the Basecamp rich text spec, which only allowshrefon<a>tags. However, the Basecamp API itself returns document content where links includetarget="_blank"andrel="noreferrer"attributes (and empirically also accepts them in write requests).This creates a silent data loss bug on round-trips: content fetched via
bc4 document view --jsonthat contains hyperlinks will havetargetandrelin its HTML. If that content is passed back todocument edit, the validator rejects it — but because the failure happens after Goldmark's safe mode has already stripped the raw HTML blocks, the result is an empty document saved with exit code 0 and no error message.Fix
Adds
NormalizeRichText()to theConverterinterface, which stripstargetandrelfrom<a>tags before running validation. This keeps the validator aligned with the spec while making round-trips work correctly.stripUnsupportedTags()is also updated to strip these attributes so the standard Markdown pipeline benefits from the same normalisation.ValidateBasecampHTMLis unchanged — it still rejectstargetandrel, which is correct per spec. Callers that need round-trip support should useNormalizeRichTextinstead.Testing