Skip to content

🐛 Fixed block scalars being added to downloaded redirects#28480

Merged
allouis merged 1 commit into
mainfrom
fabien-onc-1827-block-scalars-being-added-to-redirects
Jun 10, 2026
Merged

🐛 Fixed block scalars being added to downloaded redirects#28480
allouis merged 1 commit into
mainfrom
fabien-onc-1827-block-scalars-being-added-to-redirects

Conversation

@allouis

@allouis allouis commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

ref ONC-1827

Problem

Downloading redirects as YAML rewrote long URL values into >- folded block scalars that the user never authored:

301:
  /old/: >-
    /very/long/destination/path/that/is/definitely/longer/than/eighty/characters/

This didn't break redirects, but it injected undocumented markup into the user's file, causing confusion (reported by a customer who edited the markup out, re-uploaded, and saw Ghost add it back on the next download).

Root cause

serializeToYaml in redirect-config-parser.ts calls yaml.dump(), which defaults to js-yaml's lineWidth: 80. Any value longer than 80 chars gets folded into a block scalar.

Fix

Pass {lineWidth: -1} to disable line folding, so values are emitted verbatim on a single line. Values containing real newlines still use a literal block scalar (|-) and round-trip correctly through parseYaml.

Testing

Added a red/green unit test (does not fold long values into block scalars) asserting a long to URL produces no >- markup and round-trips intact. Verified the test fails without the fix and passes with it. All 23 redirect-config-parser tests pass.

🤖 Generated with Claude Code

ref https://linear.app/ghost/issue/ONC-1827/block-scalars-being-added-to-redirects

- downloading redirects as YAML rewrote long URL values into `>-` folded
  block scalars that the user never authored, causing confusion
- root cause: js-yaml's `yaml.dump` defaults to lineWidth 80, folding any
  line longer than that
- pass `{lineWidth: -1}` to disable folding so values are emitted verbatim;
  values with real newlines still use a literal block scalar and round-trip
  correctly through parseYaml
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5c1731d4-0d52-4f76-884a-e06a12bbaf39

📥 Commits

Reviewing files that changed from the base of the PR and between e28d63b and fa7c301.

📒 Files selected for processing (2)
  • ghost/core/core/server/services/custom-redirects/redirect-config-parser.ts
  • ghost/core/test/unit/server/services/custom-redirects/redirect-config-parser.test.ts

Walkthrough

This PR fixes YAML serialization in the redirect config parser by disabling line folding. The formatSection function now passes { lineWidth: -1 } to yaml.dump(), which prevents long URL and value strings from being rewritten into folded block scalar syntax. A new test verifies that long redirect target values are emitted without folding and that the resulting YAML parses back correctly.

Possibly related PRs

  • TryGhost/Ghost#27697: Both PRs modify YAML serialization in redirect-config-parser.ts and add unit coverage to ensure long redirect values aren't YAML-folded.

Suggested reviewers

  • vershwal
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main fix: preventing unwanted block scalar markup from being added to downloaded YAML redirects, which matches the core change in the PR.
Description check ✅ Passed The description is clearly related to the changeset, explaining the problem, root cause, fix implementation, and testing approach for the redirect configuration parser changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fabien-onc-1827-block-scalars-being-added-to-redirects

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


Comment @coderabbitai help to get the list of available commands and usage tips.

@allouis allouis self-assigned this Jun 10, 2026
@allouis allouis marked this pull request as ready for review June 10, 2026 11:11
@allouis allouis merged commit 1824a6e into main Jun 10, 2026
50 checks passed
@allouis allouis deleted the fabien-onc-1827-block-scalars-being-added-to-redirects branch June 10, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant