fix: accept dig-style argument order and reject unknown types#78
Conversation
Positional tokens matching a known RR type or class name are now interpreted as such regardless of position, so 'tdig txt sony.com' works like dig. Unknown types/classes given via -t/-c exit with a clear message instead of crashing packet creation with ArgumentError. Known type/class names are derived from tenbin_dns at compile time, so lookups never create atoms at runtime and do not depend on lazy module loading in escript mode.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
全体として、Issue #77 に対する丁寧な修正です。TDD による検証、実機での escript 検証、コンパイル時導出による atom 安全性の確保など、設計判断が明確でよく考えられています。以下に軽微な改善点を挙げます。
| cond do | ||
| type -> %{result | type: type} | ||
| class -> %{result | class: class} | ||
| result.name == nil -> %{result | name: add_tail_dot(arg)} |
There was a problem hiding this comment.
ℹ️ [LOW] assign_positional_arg/1 の add_tail_dot(arg) が name に代入されますが、txt. のような末尾ドット付きトークンでは parse_type("txt.") は nil を返すため name 扱いになる想定と読み取れます。ただし add_tail_dot の実装がこの diff に含まれていないため、末尾ドット除去後の重複ドット処理などが正しく行われるか確認をお勧めします（テストで txt. → txt. は担保されています）。
There was a problem hiding this comment.
確認しました。add_tail_dot/1 は本 PR で変更していない既存関数で、実装は「末尾が . ならそのまま返す、なければ . を1つ追加する」のみです:
def add_tail_dot(name) do
if name |> String.ends_with?(".") do
name
else
name <> "."
end
end末尾ドットの除去は行わないため重複ドットは発生せず、"txt." はそのまま "txt." になります(ご指摘のとおりテスト "trailing dot forces a type-like token to be a name" で担保済み。既存の add_tail_dot 単体テスト4件もあります)。コード変更なしで据え置きます。
| # such as "any", and a trailing dot (e.g. "txt.") forces a token to be | ||
| # interpreted as a name, both as in dig. | ||
| defp assign_positional_arg(arg, result) do | ||
| type = if result.type == nil, do: parse_type(arg) |
There was a problem hiding this comment.
ℹ️ [LOW] type = if result.type == nil, do: parse_type(arg) は else 節が無いため result.type が既に設定済みの場合 type は nil になります。意図どおり(既にtypeが埋まっていれば次のcondでclass/nameへ流れる)ですが、if の戻り値が nil になる依存は読み手に分かりにくいので、可読性向上のため意図をコメントするか明示的に else nil を記述するとよいでしょう。
There was a problem hiding this comment.
意図どおりです(type 設定済みなら nil になり、cond で class/name の判定へ流れます)。if cond, do: expr が else 省略時に nil を返すのは Elixir の言語仕様どおりの標準イディオムで、else: nil の明示は mix format / credo の慣行的にも冗長になるため追加しません。判定の意図自体は assign_positional_arg/2 直上のコメント(dig 互換の解釈順: type → class → name)で説明済みなので、コード変更なしで据え置きます。
| @type_by_name for code <- 0..0xFFFF, | ||
| type = DNS.type(code), | ||
| into: %{}, | ||
| do: {Atom.to_string(type), type} |
There was a problem hiding this comment.
✨ [POSITIVE] @type_by_name / @class_by_name を 0..0xFFFF (65536 回) 走査してコンパイル時に構築している点は、実行時オーバーヘッドが無く atom 安全性も担保される良い設計です。tenbin_dns から動的に導出することで、type/class の追加に自動追従できる保守性の高さも評価できます。
1 participant
Resolves #77
問題
tdig txt sony.comのようにクエリタイプを名前より先に指定すると、位置固定の引数解釈によりsony.comが type として atom 化され、パケット生成時にArgumentErrorでクラッシュしていた（詳細は #77）。変更内容
1. dig 互換の位置引数解釈
anyは type と class の両方に存在）は dig 同様 type 解釈を優先txt.）で type 名と同名のホストを name として指定可能（dig と同じ escape hatch）Invalid argument: <token>で明示 exit（旧: 汎用の "argument error"）2. 未知 type/class のエラーハンドリング
-t/-cスイッチに未知の値を渡すとUnknown type: foo/Unknown class: fooを stderr に出して exit 1（旧: パケット生成まで進んで stacktrace 付きクラッシュ）3. atom 安全性と escript 対応
str2atom（String.to_atom/1による任意 atom 生成）を廃止DNS.type/1/DNS.class/1を 0..0xFFFF で走査）。実行時は純粋な Map 参照のみString.to_existing_atom/1+ membership 判定で実装したが、escript は module を遅延ロードするためDNS未ロード時に:txt等の atom が存在せず誤判定する問題を実機検証で発見し、この方式に変更（テストだけでは検出できないケース）検証
TDD: 再現ケースを先にテスト化（9 件追加、実装前に red を確認）。
実機検証（escript 再ビルド後）:
./tdig txt sony.com;sony.com. IN TXT（旧: ArgumentError）./tdig TXT sony.com./tdig sony.com txt./tdig sony.com txt in./tdig any example.com./tdig txt.txt.として解釈./tdig -t foo sony.comUnknown type: foo/ exit 1./tdig -c foo sony.comUnknown class: foo/ exit 1./tdig sony.com badtypeInvalid argument: badtype/ exit 1./tdig -x 8.8.8.8/--version品質:
mix test（55 passed）/mix credo --strict（no issues）/mix format --check-formattedすべて green。README の Usage 節も順不同解釈に合わせて更新。