20260515 fe 369 기능개선fe 글 쓰기 페이지 개선#370
Merged
Hidden character warning
The head ref may contain hidden characters: "20260515-FE-369-\uae30\ub2a5\uac1c\uc120fe-\uae00-\uc4f0\uae30-\ud398\uc774\uc9c0-\uac1c\uc120"
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughRichTextEditor에 이미지/파일/비디오 업로드·드롭 처리와 미디어 툴바, 텍스트 배경색/선택 갱신을 추가하고, TipTap JSON→안전한 HTML 변환 유틸(jsonToHtml)을 도입했습니다. 게시글 첨부는 정규화·인라인 중복 필터링 후 업로드된 식별자를 합쳐 전송합니다. ChangesRichTextEditor 미디어 업로드 및 도구모음 확장
TipTap JSON → 안전한 HTML 변환 유틸
첨부 정규화·인라인 필터링·업로드 흐름
PostEditForm 및 페이지 스타일 조정
Sequence DiagramsequenceDiagram
participant User
participant Editor as RichTextEditor
participant onUploadFile as onUploadFile 콜백
participant onAttachFiles as onAttachFiles 콜백
User->>Editor: 파일 드롭/파일 선택
Editor->>Editor: 파일 타입 분류 (이미지 / 비이미지 / 비디오)
alt 이미지
Editor->>Editor: 이미지 삽입
else 비이미지 & onAttachFiles 있음
Editor->>onAttachFiles: onAttachFiles(files)
onAttachFiles-->>Editor: 메타데이터 반환
Editor->>Editor: 에디터에 메타데이터 반영(링크 등)
else 비이미지 & onAttachFiles 없음
Editor->>onUploadFile: 업로드 요청
onUploadFile-->>Editor: 업로드 결과(URL/식별자)
Editor->>Editor: 링크(escaped) 삽입
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
frontend/src/pages/BoardWrite.module.css (2)
88-90: ⚡ Quick win
.boardSelectCol중복 선언을 하나로 정리해주세요.현재 같은 선택자가 두 번 선언되어(
300px→260px) 실제 적용값이 캐스케이드 순서에 의존합니다. 의도값 하나만 남기면 레이아웃 회귀를 줄일 수 있습니다.제안 diff
.boardSelectCol { - flex: 0 0 300px; + flex: 0 0 260px; } @@ -.boardSelectCol { - flex: 0 0 260px; -}Also applies to: 304-306
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/BoardWrite.module.css` around lines 88 - 90, Duplicate .boardSelectCol rules exist with conflicting widths (300px vs 260px); remove the duplicate and consolidate into a single .boardSelectCol declaration (use the intended value — keep 260px if that is the desired layout) so the style no longer depends on cascade order; update the one in BoardWrite.module.css (and remove the other occurrence around the second reported region) to a single definition.
203-215: ⚡ Quick win삭제 버튼에
:focus-visible스타일을 추가해주세요.키보드 탐색 시 포커스 위치가 명확하지 않아 접근성이 떨어집니다. hover와 별도로 focus-visible 상태를 정의하는 것이 좋습니다.
제안 diff
.attachmentRemove:hover { color: `#ef4444`; } + +.attachmentRemove:focus-visible { + outline: 2px solid `#2b6ef3`; + outline-offset: 2px; + color: `#ef4444`; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/BoardWrite.module.css` around lines 203 - 215, Add a visible keyboard-focus style for the .attachmentRemove button by defining a .attachmentRemove:focus-visible rule (separate from :hover) that sets an accessible focus indicator (for example a clear outline, box-shadow ring, or border with sufficient contrast and preserved border-radius) and ensures outline-offset or padding so the indicator doesn't overlap content; update .attachmentRemove class if needed to avoid conflicting outlines and ensure the focus style matches the visual design and hover color semantics.frontend/src/components/Board/RichTextEditor.jsx (1)
630-643: 💤 Low value도달 불가능한 코드 (Dead code)
이
else if (onAttachFiles)분기는 실행되지 않습니다.onAttachFiles가 truthy이면 위의 554번 라인 조건에서 처리 후 589번 라인에서 반환되므로, 이 코드는onAttachFiles가 falsy일 때만 도달합니다.이 블록을 제거하여 코드 가독성을 개선하세요.
♻️ 제안된 수정
if (onUploadFile) { for (const file of otherFiles) { try { const uploaded = await onUploadFile(file); const normalizedUrl = toAbsoluteImageUrl(uploaded?.url || uploaded?.fileUrl || uploaded?.downloadUrl || uploaded?.savedUrl || file.name); insertFileLink(normalizedUrl || '', uploaded?.originalFilename || file.name); } catch (err) { console.error('파일 드롭 업로드 실패 (onUploadFile):', err); } } - } else if (onAttachFiles) { - // fallback: onAttachFiles may upload and return uploaded metadata array - try { - const uploadedArr = await onAttachFiles(otherFiles); - if (Array.isArray(uploadedArr)) { - for (const uploaded of uploadedArr) { - const normalizedUrl = toAbsoluteImageUrl(uploaded?.url || uploaded?.fileUrl || uploaded?.downloadUrl || uploaded?.savedUrl || uploaded?.url); - insertFileLink(normalizedUrl || '', uploaded?.originalFilename || uploaded?.name || '첨부파일'); - } - } - } catch (err) { - console.error('파일 드롭 업로드 실패 (onAttachFiles):', err); - } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Board/RichTextEditor.jsx` around lines 630 - 643, The else-if branch checking onAttachFiles inside the file-drop handler is dead code because onAttachFiles is already handled earlier (and the function returns), so remove the entire "else if (onAttachFiles) { ... }" block that attempts to call onAttachFiles, normalize URLs with toAbsoluteImageUrl, and call insertFileLink for uploadedArr; ensure any necessary error handling or variables (otherFiles, uploadedArr) are not referenced elsewhere after removal.frontend/src/components/Board/RichTextEditor.module.css (1)
282-286: 💤 Low value
.imageResizeHandleSE클래스 중복 선언동일한 클래스
.imageResizeHandleSE가 두 번 선언되어 있습니다 (라인 282-286과 306-310). 두 번째 선언(right: -7px; bottom: -7px)이 첫 번째 선언(right: -9px; bottom: -9px)을 덮어씁니다.의도된 값만 남기고 중복 선언을 제거하세요.
Also applies to: 306-310
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Board/RichTextEditor.module.css` around lines 282 - 286, The .imageResizeHandleSE class is declared twice (duplicate declarations where the second overrides the first); remove the redundant declaration and consolidate into a single .imageResizeHandleSE rule, preserving the intended right/bottom values (pick and keep either right: -7px; bottom: -7px or right: -9px; bottom: -9px so there is only one authoritative declaration) and keep cursor: nwse-resize in that single rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/Board/PostDetail/PostEditForm.jsx`:
- Around line 43-48: The onDrop handler handleDrop currently only calls
setIsDragOver(false) which allows the browser to navigate/open dropped files
when users drop outside the RichTextEditor; update handleDrop to call
e.preventDefault() (and optionally e.stopPropagation()) before clearing drag
state so the browser default file navigation is suppressed and the
RichTextEditor can still handle drops inside its own onDrop; locate handleDrop
in PostEditForm.jsx and add the preventDefault call while preserving the
setIsDragOver(false) behavior.
In `@frontend/src/components/Board/RichTextEditor.jsx`:
- Around line 991-1000: The color picker is reading
editor.getAttributes('textStyle')?.backgroundColor while applyTextBackground
calls toggleHighlight({ color }) (the Highlight mark stores color on the
'highlight' mark), so the picker value isn't synced to the actual highlight
color; update the input value to read the highlight mark attribute (e.g.,
editor.getAttributes('highlight')?.color) and ensure any code that reads/writes
colors uses the same mark/attribute (align applyTextBackground/toggleHighlight
and any getters to use the 'highlight' mark's color attribute instead of
textStyle.backgroundColor).
In `@frontend/src/pages/BoardWrite.jsx`:
- Around line 431-433: The current handleRemoveAttachment removes attachments by
comparing only mediaId, which causes items with undefined mediaId to all be
removed; change it to compute and compare the same fallback key used in
rendering (mediaId || url || originalFilename || name) so the removal only
removes the exact item—i.e., accept an identifier param (or keep mediaId but
treat it as the fallback id), compute const idToRemove = mediaId || url ||
originalFilename || name for the target and in the filter compute each file's id
the same way and compare equality; apply the same change to the other removal
handlers in the same area (the functions around the 500–505 range) so all
deletes use the identical fallback identifier logic.
- Around line 112-129: The link and inline-style insertion in node.marks.forEach
is vulnerable to XSS because mark.attrs.href and style tokens (mark.attrs.color,
mark.attrs.backgroundColor, mark.attrs.fontFamily, mark.attrs.fontSize) are
inserted raw into HTML; fix by validating and normalizing href with a whitelist
of safe URL schemes (e.g., http, https, mailto, tel) using URL parsing and
rejecting/omitting any value with disallowed schemes or suspicious characters
(e.g., "javascript:" or protocol-relative values), and escape/encode the final
href value before interpolation; for inline styles, validate each token against
allowed patterns/whitelists (e.g., safe color regex, allowed font-family list,
numeric fontSize bounds) and only include style declarations that pass
validation, otherwise omit them; ensure all attribute values are safely escaped
(replace quotes/angle brackets) before composing the `<a>` or `<span>` strings
so no attribute-escaping or script injection is possible.
In `@frontend/src/pages/PostDetail.jsx`:
- Around line 422-440: The jsonToHtml path currently interpolates unvalidated
href and style values into HTML (see jsonToHtml logic in PostDetail.jsx that
builds <a href="..."> and <span style="...">), creating XSS risk; extract a
shared sanitizer utility (e.g., sanitizeHref and sanitizeStyle or a single
sanitizeMarkAttrs used by jsonToHtml and the edit/save path) that enforces
allowed URL schemes (http/https/mailto), strips/escapes dangerous characters,
and validates/normalizes CSS values (color, backgroundColor, fontFamily,
fontSize) before constructing the HTML string, then replace direct usage of
mark.attrs.href and style assembly in jsonToHtml and the edit-save conversion to
call that sanitizer so both paths use the same safe logic.
---
Nitpick comments:
In `@frontend/src/components/Board/RichTextEditor.jsx`:
- Around line 630-643: The else-if branch checking onAttachFiles inside the
file-drop handler is dead code because onAttachFiles is already handled earlier
(and the function returns), so remove the entire "else if (onAttachFiles) { ...
}" block that attempts to call onAttachFiles, normalize URLs with
toAbsoluteImageUrl, and call insertFileLink for uploadedArr; ensure any
necessary error handling or variables (otherFiles, uploadedArr) are not
referenced elsewhere after removal.
In `@frontend/src/components/Board/RichTextEditor.module.css`:
- Around line 282-286: The .imageResizeHandleSE class is declared twice
(duplicate declarations where the second overrides the first); remove the
redundant declaration and consolidate into a single .imageResizeHandleSE rule,
preserving the intended right/bottom values (pick and keep either right: -7px;
bottom: -7px or right: -9px; bottom: -9px so there is only one authoritative
declaration) and keep cursor: nwse-resize in that single rule.
In `@frontend/src/pages/BoardWrite.module.css`:
- Around line 88-90: Duplicate .boardSelectCol rules exist with conflicting
widths (300px vs 260px); remove the duplicate and consolidate into a single
.boardSelectCol declaration (use the intended value — keep 260px if that is the
desired layout) so the style no longer depends on cascade order; update the one
in BoardWrite.module.css (and remove the other occurrence around the second
reported region) to a single definition.
- Around line 203-215: Add a visible keyboard-focus style for the
.attachmentRemove button by defining a .attachmentRemove:focus-visible rule
(separate from :hover) that sets an accessible focus indicator (for example a
clear outline, box-shadow ring, or border with sufficient contrast and preserved
border-radius) and ensures outline-offset or padding so the indicator doesn't
overlap content; update .attachmentRemove class if needed to avoid conflicting
outlines and ensure the focus style matches the visual design and hover color
semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29047c3a-3bdd-4604-867b-c3a8535e1c75
⛔ Files ignored due to path filters (8)
frontend/src/assets/toolbox-file-icon.svgis excluded by!**/*.svgfrontend/src/assets/toolbox-img-icon.svgis excluded by!**/*.svgfrontend/src/assets/toolbox-left-icon.svgis excluded by!**/*.svgfrontend/src/assets/toolbox-mid-icon.svgis excluded by!**/*.svgfrontend/src/assets/toolbox-right-icon.svgis excluded by!**/*.svgfrontend/src/assets/toolbox-textBackgroundColor-icon.svgis excluded by!**/*.svgfrontend/src/assets/toolbox-textcolor-icon.svgis excluded by!**/*.svgfrontend/src/assets/toolbox-video-icon.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
frontend/src/components/Board/PostDetail/PostEditForm.jsxfrontend/src/components/Board/PostDetail/PostHtmlView.jsxfrontend/src/components/Board/PostDetail/PostView.jsxfrontend/src/components/Board/RichTextEditor.jsxfrontend/src/components/Board/RichTextEditor.module.cssfrontend/src/pages/BoardWrite.jsxfrontend/src/pages/BoardWrite.module.cssfrontend/src/pages/PostDetail.jsxfrontend/src/pages/PostDetail.module.css
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
frontend/src/pages/PostDetail.jsx (1)
228-239: ⚡ Quick win인라인 첨부 제외 로직은 헬퍼로 한 번만 두는 편이 안전합니다.
같은 필터가 세 군데에 복사돼 있어서, 조건이 한 번만 바뀌어도 조회/새로고침/수정 진입 결과가 쉽게 어긋납니다.
getNonInlineAttachments(post)같은 helper로 묶어두면 유지보수가 훨씬 쉬워집니다.Also applies to: 265-274, 378-387
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/PostDetail.jsx` around lines 228 - 239, The inline-attachment filtering logic is duplicated in three places (around the blocks using normalizeAttachments, extractInlineUrlsFromContent and the filteredAttachments logic); extract this into a single helper function named getNonInlineAttachments(post) that accepts a post, calls normalizeAttachments(post) and extractInlineUrlsFromContent(post) (normalizing URLs by stripping query strings), and returns the filtered attachments array using the same rules (check att.url, att.publicPath, att.savedFilename, att.originalFilename and compare against inline URLs). Replace the three duplicated filter blocks with calls to getNonInlineAttachments(post) (e.g., where filteredAttachments is currently computed) so updates to the rule are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/pages/PostDetail.jsx`:
- Around line 422-423: The current extraction for newly uploaded file IDs only
reads u?.mediaId, which is inconsistent with other code paths that normalize IDs
using f.postAttachmentId || f.mediaId || f.id; update the upload handling that
populates uploadedNewMediaIds (the Promise.all result of
boardApi.uploadBoardFile) to normalize each upload response the same way (check
postAttachmentId, mediaId, id, etc.) and only include truthy IDs, or
throw/handle an error if an upload response contains no identifiable ID; make
the same normalization change where attachmentFiles are mapped (the
attachmentFiles.map(...).filter(Boolean) usage) so both existing and newly
uploaded attachments use the identical ID-selection logic.
In `@frontend/src/utils/richTextHtml.js`:
- Around line 164-166: The fallback in contentHtml currently only joins nodes
with node.content, causing leaf media nodes (e.g., file/video) to be dropped;
update the renderNode logic (used by contentHtml) to explicitly handle supported
media leaf node types (e.g., "file", "video") by returning appropriate HTML for
those nodes when Array.isArray(node.content) is false, while keeping the
existing branch that maps and joins node.content; ensure renderNode and any
helpers produce the expected HTML for media so contentHtml and contentJson
remain consistent.
- Around line 141-143: The heading rendering can produce `<hNaN>` when
node.attrs.level is a non-numeric string (e.g. "h2"); update the heading branch
(node.type === 'heading') so you parse/coerce the level safely (e.g. use
parseInt(node.attrs?.level, 10) or Number and then if isNaN(level) set level =
1) before applying Math.min/Math.max, ensuring level is an integer between 1 and
6; keep the rest of the return using renderNode unchanged.
- Around line 149-159: The width/height values from node.attrs are only escaped
but not validated, allowing malicious CSS like "10px; position: fixed; inset:
0;"; add a CSS length sanitizer (e.g., isValidCssLength) that permits only
numeric values and safe units (px, em, rem, %, vh, vw, etc., or "auto"), use it
to validate width and height before building
alignStyle/style/widthAttr/heightAttr, and if a value fails validation fall back
to omitting the dimension or using "auto"; update where width/height are read
(the width/height variables and the construction of style, widthAttr,
heightAttr) to call the sanitizer and only include the escaped value when it
passes.
---
Nitpick comments:
In `@frontend/src/pages/PostDetail.jsx`:
- Around line 228-239: The inline-attachment filtering logic is duplicated in
three places (around the blocks using normalizeAttachments,
extractInlineUrlsFromContent and the filteredAttachments logic); extract this
into a single helper function named getNonInlineAttachments(post) that accepts a
post, calls normalizeAttachments(post) and extractInlineUrlsFromContent(post)
(normalizing URLs by stripping query strings), and returns the filtered
attachments array using the same rules (check att.url, att.publicPath,
att.savedFilename, att.originalFilename and compare against inline URLs).
Replace the three duplicated filter blocks with calls to
getNonInlineAttachments(post) (e.g., where filteredAttachments is currently
computed) so updates to the rule are made in one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 263afdd3-4750-47c5-b47c-270a54a906b8
📒 Files selected for processing (5)
frontend/src/components/Board/PostDetail/PostEditForm.jsxfrontend/src/components/Board/RichTextEditor.jsxfrontend/src/pages/BoardWrite.jsxfrontend/src/pages/PostDetail.jsxfrontend/src/utils/richTextHtml.js
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/Board/PostDetail/PostEditForm.jsx
- frontend/src/pages/BoardWrite.jsx
- frontend/src/components/Board/RichTextEditor.jsx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/pages/BoardWrite.jsx (2)
197-199:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
heading레벨에서 NaN이 발생할 수 있습니다.
Number(node.attrs?.level)가"h2"같은 문자열이면NaN이 되어<hNaN>태그가 생성됩니다.richTextHtml.js에는 이미 수정되어 있으나 이 복사본에는 적용되지 않았습니다.수정 예시 (중복 제거 전 임시 수정)
if (node.type === 'heading') { - const level = Math.min(Math.max(Number(node.attrs?.level || 1), 1), 6); + let level = parseInt(node.attrs?.level, 10); + if (Number.isNaN(level)) level = 1; + level = Math.min(Math.max(level, 1), 6); return `<h${level}>${(node.content || []).map(renderNode).join('')}</h${level}>`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/BoardWrite.jsx` around lines 197 - 199, The heading renderer can produce NaN when node.attrs?.level is a non-numeric string (e.g., "h2"); update the heading handling in BoardWrite.jsx (the block where node.type === 'heading' that uses node.attrs?.level and renderNode) to coerce the level safely by parsing an integer and falling back to 1, then clamp it between 1 and 6 before emitting the tag; in short, replace Number(node.attrs?.level || 1) with a safe parse+fallback (e.g., parseInt with isNaN check) and keep the existing clamp and renderNode usage.
201-217:⚠️ Potential issue | 🟠 Major | ⚡ Quick win이미지
width/height에 CSS 인젝션 취약점이 있습니다.검증 없이
width/height값이style속성에 직접 삽입됩니다.100px; position: fixed; inset: 0;같은 값으로 추가 CSS 선언이 주입될 수 있습니다.richTextHtml.js에는isValidCssLength검증이 있지만 이 복사본에는 누락되었습니다.수정 예시 (중복 제거 전 임시 수정)
+ const SAFE_CSS_LENGTH_PATTERN = /^(?:\d+(?:\.\d+)?|\.\d+)(?:px|em|rem|%|vh|vw)?$/i; + const isValidCssLength = (v) => { + const raw = String(v || '').trim(); + return raw === 'auto' || SAFE_CSS_LENGTH_PATTERN.test(raw); + }; + if (node.type === 'image') { const src = String(node.attrs?.src || '').replace(/"/g, '"'); const alt = String(node.attrs?.alt || '').replace(/"/g, '"'); - const width = String(node.attrs?.width || '').trim(); - const height = String(node.attrs?.height || '').trim(); + const width = isValidCssLength(node.attrs?.width) ? String(node.attrs?.width).trim() : ''; + const height = isValidCssLength(node.attrs?.height) ? String(node.attrs?.height).trim() : 'auto';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/BoardWrite.jsx` around lines 201 - 217, The image renderer in BoardWrite.jsx embeds unsanitized width/height into the style attribute (the node.type === 'image' block using variables width, height, style, widthAttr, heightAttr), allowing CSS injection; fix by validating and normalizing width/height before use—import or reimplement the existing isValidCssLength from richTextHtml.js and only include width/height in the style string and widthAttr/heightAttr when isValidCssLength(value) returns true (otherwise omit and fall back to 'auto' or no attribute), and ensure values are stripped of trailing semicolons or unsafe characters (e.g., remove ';' and disallow additional CSS declarations) so only a single CSS length/token can be injected into style.
🧹 Nitpick comments (3)
frontend/src/utils/richTextHtml.js (3)
192-192: 💤 Low value
poster속성도 URL 프로토콜 검증을 고려해 주세요.
poster는 URL 속성이므로sanitizeHref를 통한 검증이 권장됩니다.수정 예시
- const poster = node.attrs?.poster ? ` poster="${escapeHtmlAttribute(String(node.attrs.poster).trim())}"` : ''; + const posterUrl = node.attrs?.poster ? sanitizeHref(String(node.attrs.poster).trim()) : null; + const poster = posterUrl ? ` poster="${posterUrl}"` : '';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/utils/richTextHtml.js` at line 192, The poster attribute is currently only escaped via escapeHtmlAttribute but needs URL protocol validation; update the logic around node.attrs?.poster (where poster is constructed) to pass the raw value through sanitizeHref (or your URL sanitizer) first, use the sanitized result only if non-empty/allowed, then escape it with escapeHtmlAttribute before emitting `poster="..."`; ensure you reference node.attrs?.poster and the poster variable construction so the poster is omitted when sanitizeHref rejects the URL.
99-102: 💤 Low value공백이 포함된 폰트명은 CSS에서 따옴표로 감싸야 합니다.
Noto Sans KR같은 폰트명이 따옴표 없이 출력되면 CSS 표준에 맞지 않습니다. 대부분 브라우저에서 동작하지만 엣지 케이스에서 문제가 될 수 있습니다.수정 예시
- if (fontFamily) styles.push(`font-family: ${fontFamily}`); + if (fontFamily) styles.push(`font-family: "${fontFamily}"`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/utils/richTextHtml.js` around lines 99 - 102, The fontFamily value is inserted into CSS without quotes which breaks CSS for names with spaces (e.g., "Noto Sans KR"); update the logic around the fontFamily usage (the styles.push call that currently does styles.push(`font-family: ${fontFamily}`)) to detect if fontFamily contains whitespace or special characters and wrap it in appropriate quotes (and escape any embedded quotes) before appending, so the generated CSS is always valid.
163-179: ⚡ Quick win이미지
src도 다른 미디어 노드처럼 프로토콜 검증을 적용하는 것이 좋습니다.
file,video노드는getMediaNodeUrl→sanitizeHref를 통해 프로토콜 검증을 하지만,image는escapeHtmlAttribute만 적용됩니다. 일관성 있는 보안 레이어를 위해 이미지도 동일하게 처리하는 것을 권장합니다.수정 예시
if (node.type === 'image') { - const src = escapeHtmlAttribute(node.attrs?.src || ''); + const rawSrc = node.attrs?.src || ''; + // data: URL은 이미지에서 유효하므로 별도 허용 + const isDataUrl = rawSrc.startsWith('data:image/'); + const src = isDataUrl + ? escapeHtmlAttribute(rawSrc) + : (sanitizeHref(rawSrc) || ''); + if (!src) return ''; const alt = escapeHtmlAttribute(node.attrs?.alt || '');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/utils/richTextHtml.js` around lines 163 - 179, The image node handling in the image branch should apply the same protocol sanitization as file/video nodes: instead of only using escapeHtmlAttribute for src, pass the node (or node.attrs.src) through getMediaNodeUrl and/or sanitizeHref before escaping; keep escapeHtmlAttribute for alt and other attributes and preserve existing width/height/align logic (functions referenced: getMediaNodeUrl, sanitizeHref, escapeHtmlAttribute, isValidCssLength, and the image branch handling for node.type === 'image'). Update the src assignment to use the sanitized URL result so image URLs receive the same protocol validation as other media nodes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/pages/BoardWrite.jsx`:
- Around line 26-73: Remove the duplicated sanitization and HTML conversion
implementations in BoardWrite.jsx and instead import and use the canonical
utilities from richTextHtml.js (e.g., import escapeHtmlAttribute, sanitizeUrl,
sanitizeColor, sanitizeFontFamily, sanitizeFontSize, jsonToHtml, and any helpers
like isValidCssLength/heading normalization); replace local definitions of
escapeHtmlAttribute, sanitizeUrl, sanitizeColor, sanitizeFontFamily,
sanitizeFontSize and any jsonToHtml usage with the imported functions so the
file inherits the security fixes (isValidCssLength checks and heading NaN
handling) present in richTextHtml.js and delete the redundant code blocks in
BoardWrite.jsx.
---
Outside diff comments:
In `@frontend/src/pages/BoardWrite.jsx`:
- Around line 197-199: The heading renderer can produce NaN when
node.attrs?.level is a non-numeric string (e.g., "h2"); update the heading
handling in BoardWrite.jsx (the block where node.type === 'heading' that uses
node.attrs?.level and renderNode) to coerce the level safely by parsing an
integer and falling back to 1, then clamp it between 1 and 6 before emitting the
tag; in short, replace Number(node.attrs?.level || 1) with a safe parse+fallback
(e.g., parseInt with isNaN check) and keep the existing clamp and renderNode
usage.
- Around line 201-217: The image renderer in BoardWrite.jsx embeds unsanitized
width/height into the style attribute (the node.type === 'image' block using
variables width, height, style, widthAttr, heightAttr), allowing CSS injection;
fix by validating and normalizing width/height before use—import or reimplement
the existing isValidCssLength from richTextHtml.js and only include width/height
in the style string and widthAttr/heightAttr when isValidCssLength(value)
returns true (otherwise omit and fall back to 'auto' or no attribute), and
ensure values are stripped of trailing semicolons or unsafe characters (e.g.,
remove ';' and disallow additional CSS declarations) so only a single CSS
length/token can be injected into style.
---
Nitpick comments:
In `@frontend/src/utils/richTextHtml.js`:
- Line 192: The poster attribute is currently only escaped via
escapeHtmlAttribute but needs URL protocol validation; update the logic around
node.attrs?.poster (where poster is constructed) to pass the raw value through
sanitizeHref (or your URL sanitizer) first, use the sanitized result only if
non-empty/allowed, then escape it with escapeHtmlAttribute before emitting
`poster="..."`; ensure you reference node.attrs?.poster and the poster variable
construction so the poster is omitted when sanitizeHref rejects the URL.
- Around line 99-102: The fontFamily value is inserted into CSS without quotes
which breaks CSS for names with spaces (e.g., "Noto Sans KR"); update the logic
around the fontFamily usage (the styles.push call that currently does
styles.push(`font-family: ${fontFamily}`)) to detect if fontFamily contains
whitespace or special characters and wrap it in appropriate quotes (and escape
any embedded quotes) before appending, so the generated CSS is always valid.
- Around line 163-179: The image node handling in the image branch should apply
the same protocol sanitization as file/video nodes: instead of only using
escapeHtmlAttribute for src, pass the node (or node.attrs.src) through
getMediaNodeUrl and/or sanitizeHref before escaping; keep escapeHtmlAttribute
for alt and other attributes and preserve existing width/height/align logic
(functions referenced: getMediaNodeUrl, sanitizeHref, escapeHtmlAttribute,
isValidCssLength, and the image branch handling for node.type === 'image').
Update the src assignment to use the sanitized URL result so image URLs receive
the same protocol validation as other media nodes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3a5e1b61-fb01-44a0-a2e2-2e790dd90f13
📒 Files selected for processing (3)
frontend/src/pages/BoardWrite.jsxfrontend/src/pages/PostDetail.jsxfrontend/src/utils/richTextHtml.js
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/PostDetail.jsx
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
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 by CodeRabbit
새로운 기능
개선 사항
버그 수정