Skip to content

Fix Ctrl+S save behavior for opened files in rhwp-studio#180

Closed
ahnbu wants to merge 16 commits into
edwardkim:develfrom
ahnbu:local/task179-devel
Closed

Fix Ctrl+S save behavior for opened files in rhwp-studio#180
ahnbu wants to merge 16 commits into
edwardkim:develfrom
ahnbu:local/task179-devel

Conversation

@ahnbu

@ahnbu ahnbu commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • prefer showOpenFilePicker() when opening a document so rhwp-studio can retain the original file handle
  • reuse the existing file handle on Ctrl+S and only fall back to save picker or download when no handle exists
  • add focused tests for the new file-system access helper

Root Cause

The previous open flow used <input type="file">, which only exposes File.name and not the original path or writable handle. As a result, Ctrl+S always had to open a save dialog and the browser defaulted to its own last-used folder.

Verification

  • node --experimental-strip-types --test tests/file-system-access.test.ts
  • npm run build
  • browser smoke check on http://127.0.0.1:7700: showOpenFilePicker path saved through createWritable() with savePickerCalls=0

Closes #179.

edwardkim and others added 15 commits April 13, 2026 21:58
Release v0.7.2: VS Code 컨텍스트 메뉴 + 한컴 단축키 + 커맨드 팔레트 + 양식 컨트롤
Release v0.7.2: npm 패키지 배포 준비 — @rhwp/core + @rhwp/editor + VS Code 0.7.2
Windows 환경에서 cfb 크레이트 동작 시 발생하는 역슬래시(\) 경로 구분자 문제를 슬래시(/) 단위로 일일이 변경하여 모든 OS 환경에서 일관된 경로를 보장하도록 함.
Problem
-------
HwpxReader::read_file and read_file_bytes called read_to_string /
read_to_end directly on zip::ZipFile readers with no size ceiling.
ZIP allows extreme compression ratios, so a few-KB .hwpx can claim a
single section.xml entry that expands to multi-GB — OOMing the host.
Any downstream (CLI, WASM, browser extension, VS Code) inherits the
risk since all of them funnel through parse_hwpx → HwpxReader.

Fix
---
Introduce a small read_limited(reader, max) helper using Read::take
so we never allocate past max+1 bytes. Apply:

  - MAX_XML_SIZE     = 32 MB  for text entries (section/header/hpf)
  - MAX_BINDATA_SIZE = 64 MB  for binary entries (images, fonts)

Real-world Korean government HWPX (보도자료·법령·판례) stays well under
these, so legitimate files are unaffected. Over-limit entries surface
as HwpxError::ZipError with "decompression bomb" in the message —
callers see a clean parse error instead of a crash.

Rationale for thresholds
------------------------
MDM (https://github.com/seunghan91/markdown-media) has been running
the same 32/64 MB caps in production across ~500 real HWPX fixtures
with zero false positives; see MDM commit a94b459 for the original
application of this pattern across HWP/HWPX/PDF parser paths.

Tests
-----
Five unit tests in parser::hwpx::reader:
  - under / at / over cap boundary for read_limited
  - a synthetic zip-bomb entry (MAX_XML_SIZE + 1 bytes of 'A',
    compressed to <1 MB) is rejected with ZipError

Full suite: 789 passed, 0 failed (baseline + 5 new).

No public API change: HwpxError variants unchanged, method signatures
unchanged. MAX_XML_SIZE / MAX_BINDATA_SIZE are exposed as pub const
so integrators can read the current caps.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Problem
-------
parser/hwpx/header.rs:293 treats any shape != "NONE" | "3D" as a real
strikethrough:

    cs.strikethrough = !matches!(val.as_str(), "NONE" | "3D");

This is a blacklist of known placeholders. It is correct for the 3D
case Hancom emits today, but fails open: any future placeholder value
Hancom adds (e.g. "4D", "Ghost", internal experiments) will be
misinterpreted as a real strike line, and entire body-text paragraphs
will render with strikethrough.

Fix
---
Flip the predicate to a whitelist of OWPML LineSym2 values that shape.rs
already recognises (SOLID, DASH, DOT, DASH_DOT, DASH_DOT_DOT, LONG_DASH,
CIRCLE, DOUBLE_SLIM, SLIM_THICK, THICK_SLIM, SLIM_THICK_SLIM, WAVE,
DOUBLE_WAVE — 13 entries, same set as the enum mapping immediately
below). Unknown values — including NONE, 3D, and any future Hancom
placeholder — are fail-closed to no-strike.

Extracted into a `pub(crate) fn is_real_strike_shape(&str) -> bool`
so the predicate is independently testable and can be reused if other
parsers (e.g. underline, which has the same blacklist risk) adopt the
same approach later.

Evidence
--------
MDM (https://github.com/seunghan91/markdown-media) hit the exact same
bug earlier this month. Hancom's HWPX exporter writes <hh:strikeout
shape="3D"/> as a placeholder default on body-text charPr definitions.
Treating "anything but NONE" as strike caused press-release bodies
(e.g. "251113 벤처투자 보도자료") to render wrapped in ~~...~~.

Our fix (MDM commit ae15dd8) moved to the whitelist approach and
verified across 21 MOIS HWPX fixtures: zero false strikes remained.
The same logic fits rhwp one-for-one, so we're upstreaming it.

Tests
-----
Five new unit tests in parser::hwpx::header::tests:
  - all 13 valid OWPML LineSym2 shapes → true
  - "NONE" → false
  - "3D" → false (the original bug)
  - fail-closed cases: "4D", "Ghost", "", "solid" (case-sensitive)

Full suite: 789 passed, 0 failed.

No behaviour change for files Hancom currently emits — the set of
shapes that produce strike=true is identical to the old blacklist
path for every value shape.rs knows about. The only difference is
forward-compatibility: unknown future placeholders no longer get
mis-rendered as strike.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eparator

fix(parser): Windows 환경의 CFB 경로 구분자 오류 수정
…otection

fix(hwpx): cap ZIP entry decompression to defeat zip bombs
…-whitelist

refactor(hwpx): whitelist strikeout shapes instead of blacklist
- showOpenFilePicker로 연 파일 handle을 보관하고 Ctrl+S 시 같은 파일에 바로 덮어쓰기
- handle이 없는 input 파일 열기 경로는 save picker와 download fallback 유지
- file-system-access 유틸과 단위 테스트 추가

Co-Authored-By: Codex <noreply@openai.com>
- add PWA manifest, launchQueue file opening, and preview save bridge
- add Windows launcher scripts, Go wrapper, and bundled RHWPLauncher.exe
- cover preview server and file-system access flows with tests

Co-Authored-By: Codex <noreply@openai.com>
- default user-requested docs to the repository root _docs directory
- document mydocs as a legacy/internal location instead of the default output path

Co-Authored-By: Codex <noreply@openai.com>
- launcher manifest를 UTF-8 without BOM으로 기록\n- preview server가 BOM manifest를 호환 파싱\n- save trace와 명시적 저장 실패 노출을 추가

Co-Authored-By: Codex <noreply@openai.com>
- 오류 원인을 manifest BOM 경로로 확정\n- 수정 내역과 재검증 결과를 보고서에 반영\n- 실제 사용자 확인 결과를 문서에 추가

Co-Authored-By: Codex <noreply@openai.com>
- 글로벌 BACKLOG 템플릿으로 루트 BACKLOG.md 생성\n- 폰트 변경 후 Ctrl+Z 실행취소 개선 항목을 BL-0417-01로 등록

Co-Authored-By: Codex <noreply@openai.com>
@edwardkim

Copy link
Copy Markdown
Owner

@ahnbu 기여 감사합니다! Ctrl+S 파일 핸들 유지 아이디어는 좋습니다.

다만 몇 가지 확인이 필요합니다:

1. 변경 범위

  • CLAUDE.md, AGENTS.md, BACKLOG.md, CHANGELOG.md 수정은 메인테이너가 관리하는 파일입니다. 이 파일들의 변경은 제거해주세요.
  • _docs/ 폴더는 프로젝트 문서 규칙(mydocs/)과 다릅니다. 제거해주세요.
  • PWA manifest, 로고 파일(logo-192.png, logo-512.png)은 이 PR의 범위와 다릅니다. 별도 PR로 분리해주세요.

2. 충돌 가능성

3. 요청

  • PR 범위를 Ctrl+S 파일 핸들 유지 + file-system-access 모듈로 한정해주세요.
  • 그 외 변경(문서, PWA, 로고)은 별도 PR로 분리해주세요.

감사합니다!

- 현재 브랜치의 업스트림 PR 가능 범위를 분석 문서로 저장\n- 1차 PR 후보와 Windows 경로 blocker를 분리 정리

Co-Authored-By: Codex <noreply@openai.com>
@ahnbu

ahnbu commented Apr 18, 2026

Copy link
Copy Markdown
Contributor Author

@edwardkim 피드백 감사합니다!

지적하신 내용을 모두 반영하여 정리했습니다.

제거한 항목:

  • CLAUDE.md, AGENTS.md, BACKLOG.md, CHANGELOG.md — 메인테이너 관리 파일 제외
  • _docs/ 폴더 — 로컬 문서 제외
  • rhwp-studio/public/logo-*.png, manifest.webmanifest — PWA 관련, 별도 PR로 분리
  • tools/ (Windows 런처, Go wrapper, 설치 스크립트) — 별도 PR로 분리

충돌 해결:

  • upstream/devel 기준으로 rebase하여 file.ts 충돌 해결했습니다.

새 클린 브랜치 pr/task179를 준비했으며, 변경 파일은 rhwp-studio/ 내부 5개 파일로 한정됩니다. 동작 테스트 확인 후 별도 PR로 올리겠습니다. 이 PR은 close합니다.

@ahnbu ahnbu closed this Apr 18, 2026
ahnbu added a commit to ahnbu/rhwp that referenced this pull request Apr 18, 2026
- Claude 세션의 PR edwardkim#180 대응 내역과 7커밋 분석을 문서화
- clean PR 상태와 WSL Docker 검증 계획을 함께 정리

Co-Authored-By: Claude <noreply@anthropic.com>
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.

4 participants