Interpolate Maven ${...} properties through one formats::maven helper: five copies drifted on recursion, undefined names and the length cap

オープン
  • 何か問題がありますか？
#1,887 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
1〜2日
初心者へのやさしさ
55/100
issue の種類
リファクタリング
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
rust
領域
backend, devtools

調査の方向性

Start with the interpolation helpers in crates/socket-patch-core/src/formats/maven/mod.rs and the four call sites in maven_crawler.rs, maven_scope.rs, and vendor/jvm/maven_reactor.rs. Read the existing tests for those paths and maven_pom_equivalence_tests.rs before consolidating the lookup loop. Done means in-cap behavior stays unchanged and a doubling property chain is refused in both scope and reactor tests.

索引モデルが issue の本文から書いたものです。

説明

agent:triaged arch-audit priority:p3 refactor

[agent] Filed by the scheduled architecture audit (audit-core). Register: register.

Kind: refactor. Register ID: A30.

Problem

The ${name} substitution loop (find("${"), find('}'), lookup, push) is written five times. The copies have drifted:

Copy Recursive Undefined name Output length cap
maven_crawler::interpolate L738-L751 (settings.xml) no None none
formats::maven::interpolate / interpolate_once L329-L372 up to MAX_INTERPOLATION rounds kept verbatim MAX_INTERPOLATED_LEN
maven_scope::interpolate_at L491-L534 depth-limited None none
maven_reactor::interpolate_by L1002-L1021 depth-limited None none
maven_reactor::Reactor::interpolate_with L1689-L1710 depth-limited None none

The last three are the same loop body, differing only in how they call the lookup:

while let Some(at) = rest.find("${") {
    out.push_str(&rest[..at]);
    let after = &rest[at + 2..];
    let close = after.find('}')?;
    let raw = lookup(&after[..close])?;
    out.push_str(&interpolate_by(&raw, lookup, depth + 1)?);
    rest = &after[close + 1..];
}

Only formats::maven caps the output length. The depth-limited recursive copies can still grow exponentially within the depth limit (a=${b}${b}, b=${c}${c}, …), which is the "billion laughs" case that cap exists for. This is from reading the code; I did not execute it.

Proposed change
  • Add formats::maven::interpolate_by(value, lookup: &dyn Fn(&str) -> Option<String>) -> Option<String>. It recurses with MAX_INTERPOLATION as the depth limit and returns None past MAX_INTERPOLATED_LEN.
  • maven_scope, maven_reactor (both) and maven_crawler keep their lookup closures and call it.
  • maven_crawler's settings.xml use is non-recursive today. Maven itself interpolates settings.xml from env/system properties only, so recursion there is harmless, but keep a test that pins its current result.
  • formats::maven::interpolate's keep-unresolved mode becomes a lookup that echoes ${name} back, so it can share the loop too. If that changes its output, it can stay as the one separate wrapper.
Size

4 files, about −70 lines.

Acceptance criteria
  • Behavior is unchanged for in-cap inputs. These must stay green: the existing maven_scope, maven_reactor and formats::maven interpolation tests, and maven_pom_equivalence_tests.rs.
  • Add one test showing that a doubling property chain returns None (refused) in the scope and reactor paths instead of growing past MAX_INTERPOLATED_LEN.
主要言語
Rust
スター
8
フォーク
2
平均マージ
19時間 24分
マージ済み PR（30日）
669

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

SocketDev/socket-patch のほかの issue

SocketDev/socket-patch の issue をすべて見る

似ている issue

Rust の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。