diff --git a/REPORT.md b/REPORT.md index 6535e57c..8374e4b9 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,4 +1,9 @@ # REPORT +## [2026-09-23] 클래스 파일 거부가 «어디서» 걸렸는지 말한다 — 검증 규칙 11개, 오류 변종은 1개 (rustjava-2026-09-18-bootstrap-argument-index-and-tag-adopt-p0) +- 무엇을: `validate_class` 규칙을 전수 세어(14개 · 고정문장 13개) 표를 걸으며 멈춘 위치를 이미 쥔 **11개**가 그 위치를 싣게 했다 — `ClassFileError::InvalidFormatAt { cause, location }` 하나와 표 5종(`Location`)으로. +- 왜: #73 이 부트스트랩 인자 규칙 하나를 구조화한 뒤 나머지가 몇이나 되는지 아무도 세지 않았다. 규칙마다 변종을 늘리면 `&'static str` 설계가 피하던 enum 비대가 오므로, 변종은 «규칙 수»가 아니라 «표 종류 수»로만 늘게 멈춤 기준을 먼저 세웠다. +- 사용자 영향: `ClassFormatError` 문면 끝에 `(constant pool entry #18)`·`(method #2)` 처럼 위치가 붙는다. 종전 문장은 그대로 앞에 남는다. 받아들이는/거부하는 파일은 하나도 바뀌지 않는다. +- 후속 추천: `class.rs` 의 파싱 단계 거부 3종(잘림·꼬리 바이트·45.0 미만)은 이번 범위 밖 — 그중 위치를 쥔 것이 있는지만 세어 볼 것(S). 상세 = `docs/worklog/2026-09-23-validation-rules-name-their-position.{md,json}`. ## [2026-09-23] charset 보류 판단을 `Charset` 의 exhaustive match 로 옮겼다 (rustjava-2026-09-23-stale-next-pointer-and-euc-kr-boundary-adopt-p0) - 무엇을: `InputStreamReader::read()` 의 charset 이름 문자열 비교 2곳을 `Charset::bytes_to_hold_back` 로 옮겼다(wildcard 없는 match · 동작 불변). - 왜: 채택 제안 `2026-09-23-stale-next-pointer-and-euc-kr-boundary#p0` — 새 charset 을 더하면 그 if 사슬은 조용히 빠졌다. 이제 컴파일이 막는다. diff --git a/STATE.md b/STATE.md index 18ab9a93..2a602022 100644 --- a/STATE.md +++ b/STATE.md @@ -7,6 +7,12 @@ (둘 다 이것보다 오래됐고 MERGEABLE/CONFLICTING 처분이 이미 걸려 있다). 겹침은 전부 **append 형 합집합**이라 해소는 기계적이다) ## 완료 +- [rustjava-2026-09-18-bootstrap-argument-index-and-tag-adopt-p0] ★**검증 규칙이 멈춘 위치를 말한다 — 11규칙, 변종 1개.** 채택 제안 `2026-09-18-bootstrap-argument-index-and-tag#p0`. + ★**전제 반증(작게)**: `validate_class` 규칙은 15/14 가 아니라 **14 · 고정문장 13**(@origin/main `c654ae2e`). + ★**전수**: 13 중 **11**이 표를 걸으며 멈춘 위치를 쥐고 있었다(pool 3 · field 3 · method 3 · interface 1 · class attribute 1) · `this_class`/`super_class` 2개는 가리킬 곳 없음 → `InvalidFormat` 유지. + ★**멈춤 기준**: enum 은 «규칙»이 아니라 «표 종류»로만 자란다 ⇒ `InvalidFormatAt { cause, location: Location }` **1변종** + `Location` 5종. `ClassFileError` 크기 불변(기존 크기 테스트 무수정 통과). + ★**문면** = `<종전 문장> (<표> #)` · pool 은 `javap` 의 1-기반 `#N` · 파일 바이트 0. 경계 무이동(술어 본문 불변 · `all/any` → 첫 위반 위치). + ★**양방향**: 인덱스 3종 개악(M1 문면에서 위치 삭제 · M2 field/method 0 고정 · M3 pool 첫 키 보고) **전건 red** · pool 인덱스(#11·#18·#34)는 **독립 바이트 워커로 교차 확인**. - [rustjava-2026-09-23-stale-next-pointer-and-euc-kr-boundary-adopt-p0] charset 보류 판단을 `Charset::bytes_to_hold_back`(wildcard 없는 match)로 옮김 · `read()` 이름 비교 0 · 동작 불변 · 변이 양방향 확인. 채택 `2026-09-23-stale-next-pointer-and-euc-kr-boundary#p0`. - [rustjava-2026-09-23-stale-next-pointer-and-euc-kr-boundary-adopt-p1] `## 다음` 정본 결정 = ⒝ 얇은 층(ref + 선행 사슬 + 카드 밖 항목만 · 산문 지목 금지). ⒞ 기각 근거 = tower 술어로 열린 카드 0(32건 전건 injected). 채택 `2026-09-23-stale-next-pointer-and-euc-kr-boundary#p1`. 상세 `docs/worklog/2026-09-23-next-section-canon-decision.md`. - [rustjava-prune-declined-followup-proposals-2026-09-21] ★**추천 후속작업 2건 기각** — 운영자 지시(2026-09-21 우선순위 정리). ★제품 코드 **0줄** · 새 제안 **0** · 검사기/CI 신설 **0**. diff --git a/classfile/src/error.rs b/classfile/src/error.rs index 6465dfba..7f9da389 100644 --- a/classfile/src/error.rs +++ b/classfile/src/error.rs @@ -13,9 +13,10 @@ pub enum ClassFileError { /// A bootstrap method argument that is not a loadable constant, carrying the two things the /// predicate already knows at the moment it refuses: *which* argument, and what it found there. /// - /// ★ Which variant to use: `InvalidFormat` is for a rule that has nothing to point at, and that - /// is still most of them. Use this one only when a number is already in hand — folding it into - /// prose is the thing this variant exists to stop. `UnsupportedVersion` is the same shape and + /// ★ Which variant to use: `InvalidFormat` is for a rule that has nothing to point at; + /// `InvalidFormatAt` for a rule that stopped at one position in one table; this one for the + /// bootstrap-argument rule, which holds two indices and what it found. Folding a number already + /// in hand into prose is the thing these variants exist to stop. `UnsupportedVersion` is the same shape and /// predates it, so this is the established way here rather than a second scheme. /// /// It stays `Copy`: two `u16`s and a `&'static str`, no owned data. @@ -31,5 +32,43 @@ pub enum ClassFileError { /// and the message says which. actual: Option<&'static str>, }, + /// A rule that walks one of the class file's tables and stopped at a known position. + /// + /// ★ One variant for all of them, not one per rule. Eleven of `validate_class`'s rules hold a + /// position when they refuse, and a variant each is the enum growth the `&'static str` design + /// was avoiding. What differs between them is only *which table* the number indexes, and there + /// are five tables — so the table is the enum, and the rule stays the sentence it already was. + /// `InvalidBootstrapArgument` stays separate because it carries a second index and what it found. + InvalidFormatAt { + cause: &'static str, + location: Location, + }, UnsupportedVersion(u16), } + +/// Where in the class file an `InvalidFormatAt` rule stopped. +/// +/// The constant pool uses its own 1-based index — the `#N` that `javap -v` prints — because that +/// is how every tool names a pool entry. The others are zero-based positions in their table, the +/// same convention `InvalidBootstrapArgument` uses. Only an index: nothing from the file's bytes +/// (a name, a descriptor) is carried, so a hostile file cannot put text into the message. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum Location { + ConstantPoolEntry(u16), + Interface(u16), + Field(u16), + Method(u16), + ClassAttribute(u16), +} + +impl core::fmt::Display for Location { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::ConstantPoolEntry(index) => write!(f, "constant pool entry #{index}"), + Self::Interface(index) => write!(f, "interface #{index}"), + Self::Field(index) => write!(f, "field #{index}"), + Self::Method(index) => write!(f, "method #{index}"), + Self::ClassAttribute(index) => write!(f, "class attribute #{index}"), + } + } +} diff --git a/classfile/src/lib.rs b/classfile/src/lib.rs index afb1a38a..d19ec319 100644 --- a/classfile/src/lib.rs +++ b/classfile/src/lib.rs @@ -15,7 +15,7 @@ pub use { attribute::{AttributeInfo, AttributeInfoCode, BootstrapMethod, MethodHandleKind, MethodHandleRef, method_type_descriptor}, class::ClassInfo, constant_pool::{ConstantPoolReference, FieldMethodref}, - error::ClassFileError, + error::{ClassFileError, Location}, field::FieldInfo, method::MethodInfo, opcode::{LambdaCallSite, Opcode, StringConcatCallSite}, diff --git a/classfile/src/validation.rs b/classfile/src/validation.rs index effa541a..3485a2e2 100644 --- a/classfile/src/validation.rs +++ b/classfile/src/validation.rs @@ -2,7 +2,7 @@ use alloc::collections::BTreeMap; use jvm_types::MethodAccessFlags; -use crate::{AttributeInfo, ClassFileError, ClassInfo, ConstantPoolReference, constant_pool::ConstantPoolItem}; +use crate::{AttributeInfo, ClassFileError, ClassInfo, ConstantPoolReference, Location, constant_pool::ConstantPoolItem}; enum MemberKind { Field, @@ -20,6 +20,9 @@ enum MemberKind { /// The strings are the message, so they are written the way a JVM writes one. They are not /// identifiers and nothing matches on them; tests assert them to pin *which* rule fired, which is /// the observability the flat version could not give. +/// +/// A rule that walks a table reports *where* it stopped as well (`InvalidFormatAt`); the two that +/// check a single name (`this_class`, `super_class`) have nothing to point at and stay `InvalidFormat`. pub(crate) fn validate_class(class: &ClassInfo) -> Result<(), ClassFileError> { if !is_internal_class_name(&class.this_class) { return Err(ClassFileError::InvalidFormat("this_class does not name a class")); @@ -27,31 +30,39 @@ pub(crate) fn validate_class(class: &ClassInfo) -> Result<(), ClassFileError> { if class.super_class.as_ref().is_some_and(|name| !is_internal_class_name(name)) { return Err(ClassFileError::InvalidFormat("super_class does not name a class")); } - if class.interfaces.iter().any(|name| !is_internal_class_name(name)) { - return Err(ClassFileError::InvalidFormat("an interface entry does not name a class")); + if let Some(index) = class.interfaces.iter().position(|name| !is_internal_class_name(name)) { + return Err(at("an interface entry does not name a class", Location::Interface(index as u16))); } - if !validate_constant_pool(&class.constant_pool) { - return Err(ClassFileError::InvalidFormat("a constant pool entry names a missing or wrong-kind entry")); + if let Some(index) = validate_constant_pool(&class.constant_pool) { + return Err(at( + "a constant pool entry names a missing or wrong-kind entry", + Location::ConstantPoolEntry(index), + )); } - if !constant_pool_tags_fit_the_class_file_version(class) { - return Err(ClassFileError::InvalidFormat( + if let Some(index) = constant_pool_tags_fit_the_class_file_version(class) { + return Err(at( "class file version does not support a constant tag it carries", + Location::ConstantPoolEntry(index), )); } // The rule is wider than the function name: the docstring below says the argument must also be a // loadable constant, and OpenJDK says the same ("bad constant type"). The name stayed behind when // the rule widened; renaming it is not this round's scope. bootstrap_method_static_arguments_are_in_the_pool(class)?; - if !bootstrap_method_indices_resolve(class) { - return Err(ClassFileError::InvalidFormat("a dynamic constant names no bootstrap method")); + if let Some(index) = bootstrap_method_indices_resolve(class) { + return Err(at("a dynamic constant names no bootstrap method", Location::ConstantPoolEntry(index))); } - if !at_most_one_of_each_single_class_attribute(class) { - return Err(ClassFileError::InvalidFormat("a single-valued class attribute appears more than once")); + if let Some(position) = at_most_one_of_each_single_class_attribute(class) { + return Err(at( + "a single-valued class attribute appears more than once", + Location::ClassAttribute(position as u16), + )); } - for field in &class.fields { + for (index, field) in class.fields.iter().enumerate() { + let here = Location::Field(index as u16); if !is_field_descriptor(&field.descriptor) { - return Err(ClassFileError::InvalidFormat("a field descriptor is malformed")); + return Err(at("a field descriptor is malformed", here)); } let constant_values = field @@ -64,7 +75,7 @@ pub(crate) fn validate_class(class: &ClassInfo) -> Result<(), ClassFileError> { .collect::>(); // Two rules, not one: "how many" and "of what type". The flat version could not say which. if constant_values.len() > 1 { - return Err(ClassFileError::InvalidFormat("multiple ConstantValue attributes on a field")); + return Err(at("multiple ConstantValue attributes on a field", here)); } if constant_values.first().is_some_and(|value| { !matches!( @@ -76,13 +87,14 @@ pub(crate) fn validate_class(class: &ClassInfo) -> Result<(), ClassFileError> { | ("Ljava/lang/String;", ConstantPoolReference::String(_)) ) }) { - return Err(ClassFileError::InvalidFormat("a ConstantValue does not match its field descriptor")); + return Err(at("a ConstantValue does not match its field descriptor", here)); } } - for method in &class.methods { + for (index, method) in class.methods.iter().enumerate() { + let here = Location::Method(index as u16); if !is_method_descriptor(&method.descriptor) { - return Err(ClassFileError::InvalidFormat("a method descriptor is malformed")); + return Err(at("a method descriptor is malformed", here)); } let code_attributes = method @@ -92,16 +104,26 @@ pub(crate) fn validate_class(class: &ClassInfo) -> Result<(), ClassFileError> { .count(); if method.access_flags.intersects(MethodAccessFlags::ABSTRACT | MethodAccessFlags::NATIVE) { if code_attributes != 0 { - return Err(ClassFileError::InvalidFormat("an abstract or native method carries a Code attribute")); + return Err(at("an abstract or native method carries a Code attribute", here)); } } else if code_attributes != 1 { - return Err(ClassFileError::InvalidFormat("a method does not have exactly one Code attribute")); + return Err(at("a method does not have exactly one Code attribute", here)); } } Ok(()) } +fn at(cause: &'static str, location: Location) -> ClassFileError { + ClassFileError::InvalidFormatAt { cause, location } +} + +/// The key of the first pool entry `is_valid` refuses. Pool rules are "every entry satisfies X", +/// and the entry that does not is the one to name — `all` would answer the same question and drop it. +fn first_invalid_entry(constant_pool: &BTreeMap, mut is_valid: impl FnMut(&ConstantPoolItem) -> bool) -> Option { + constant_pool.iter().find(|(_, item)| !is_valid(item)).map(|(index, _)| *index) +} + /// JVMS 4.4: a constant kind is legal only from the class file version that introduced it. /// /// This lives here rather than in `validate_constant_pool` because it needs `major_version`, @@ -115,8 +137,8 @@ pub(crate) fn validate_class(class: &ClassInfo) -> Result<(), ClassFileError> { /// nothing in its place, and the class went from "corrupt" to "this runtime does not support /// that yet" — a sentence this lineage exists to make true, applied to a file no JVM can read. /// `test-data/ldc/LdcDynamicOldMajor.class` holds that case. -fn constant_pool_tags_fit_the_class_file_version(class: &ClassInfo) -> bool { - class.constant_pool.values().all(|item| { +fn constant_pool_tags_fit_the_class_file_version(class: &ClassInfo) -> Option { + first_invalid_entry(&class.constant_pool, |item| { let minimum_major_version = match item { // Java 7 (JSR 292) introduced the method handle family. ConstantPoolItem::MethodHandle { .. } | ConstantPoolItem::MethodType { .. } | ConstantPoolItem::InvokeDynamic { .. } => 51, @@ -238,13 +260,13 @@ fn constant_kind_name(item: &ConstantPoolItem) -> &'static str { /// was `UnsupportedOperationException`, i.e. *this runtime cannot do that yet*, about a file no /// runtime can read. That sentence is what the lineage exists to make true, so narrowing it here /// is the point rather than a side effect. -fn bootstrap_method_indices_resolve(class: &ClassInfo) -> bool { +fn bootstrap_method_indices_resolve(class: &ClassInfo) -> Option { let bootstrap_method_count = class.attributes.iter().find_map(|attribute| match attribute { AttributeInfo::BootstrapMethods(methods) => Some(methods.len()), _ => None, }); - class.constant_pool.values().all(|item| { + first_invalid_entry(&class.constant_pool, |item| { let index = match item { ConstantPoolItem::Dynamic { bootstrap_method_attr_index, .. @@ -297,7 +319,7 @@ fn bootstrap_method_indices_resolve(class: &ClassInfo) -> bool { /// (`ConstantValue`, `Code`, `Exceptions`, `MethodParameters`, `StackMapTable`, `LineNumberTable`, /// `LocalVariableTable`) are not listed even when they turn up in a class's attribute table, because /// there they are attributes in a place they are not defined for — ignored, not counted. -fn at_most_one_of_each_single_class_attribute(class: &ClassInfo) -> bool { +fn at_most_one_of_each_single_class_attribute(class: &ClassInfo) -> Option { // (discriminant, the class file version that introduced the attribute) fn single_valued(attribute: &AttributeInfo) -> Option<(u8, u16)> { Some(match attribute { @@ -311,23 +333,23 @@ fn at_most_one_of_each_single_class_attribute(class: &ClassInfo) -> bool { }) } - class.attributes.iter().enumerate().all(|(position, attribute)| { - let Some((kind, introduced_in)) = single_valued(attribute) else { - return true; + // The position returned is the second occurrence — the one that makes it a duplicate. + (0..class.attributes.len()).find(|&position| { + let Some((kind, introduced_in)) = single_valued(&class.attributes[position]) else { + return false; }; if class.major_version < introduced_in { - return true; + return false; } - // Only the first of each kind looks behind it, so one duplicate is reported once. - !class.attributes[..position] + class.attributes[..position] .iter() .any(|earlier| single_valued(earlier).is_some_and(|(earlier_kind, _)| earlier_kind == kind)) }) } -fn validate_constant_pool(constant_pool: &BTreeMap) -> bool { - constant_pool.values().all(|item| match item { +fn validate_constant_pool(constant_pool: &BTreeMap) -> Option { + first_invalid_entry(constant_pool, |item| match item { ConstantPoolItem::Class { name_index } => constant_pool .get(name_index) .and_then(ConstantPoolItem::utf8) diff --git a/classfile/tests/test.rs b/classfile/tests/test.rs index e7e05e1a..3437b9c9 100644 --- a/classfile/tests/test.rs +++ b/classfile/tests/test.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use jvm_types::ClassAccessFlags; -use classfile::{AttributeInfo, BootstrapMethod, ClassFileError, ClassInfo, ConstantPoolReference, MethodHandleKind, Opcode}; +use classfile::{AttributeInfo, BootstrapMethod, ClassFileError, ClassInfo, ConstantPoolReference, Location, MethodHandleKind, Opcode}; #[test] fn test_hello() { @@ -209,14 +209,20 @@ fn test_class_info_validation_rejects_invalid_names_descriptors_and_code_layout( invalid_descriptor.methods[0].descriptor = "(V)V".to_string().into(); assert_eq!( invalid_descriptor.validate(), - Err(ClassFileError::InvalidFormat("a method descriptor is malformed")) + Err(ClassFileError::InvalidFormatAt { + cause: "a method descriptor is malformed", + location: Location::Method(0), + }) ); let mut missing_code = ClassInfo::parse(hello).unwrap(); missing_code.methods[0].attributes.clear(); assert_eq!( missing_code.validate(), - Err(ClassFileError::InvalidFormat("a method does not have exactly one Code attribute")) + Err(ClassFileError::InvalidFormatAt { + cause: "a method does not have exactly one Code attribute", + location: Location::Method(0), + }) ); } @@ -320,7 +326,11 @@ fn test_bootstrap_method_reference_kinds_outside_the_set_and_mispaired_kinds_are for reference_kind in [1u8, 4, 9] { assert_eq!( parse_with_kind(reference_kind), - Some(ClassFileError::InvalidFormat("a constant pool entry names a missing or wrong-kind entry")), + Some(ClassFileError::InvalidFormatAt { + cause: "a constant pool entry names a missing or wrong-kind entry", + // the mutated MethodHandle itself — byte 383 of the fixture is pool entry #34 + location: Location::ConstantPoolEntry(34), + }), "reference kind {reference_kind} does not pair with a Methodref — and the cause says it was validation, \ not the parser, that refused it (the loop above is the parser's)" ); @@ -460,3 +470,73 @@ fn test_the_structured_variant_does_not_grow_the_error_type() { fn assert_copy() {} assert_copy::(); } + +/// Every table kind a rule can stop in, each asserted with the position the rule stopped at. +/// +/// ★ The mutations put the fault at a position that is *not* zero where the table allows it, so a +/// rule that reported "the first one" instead of "the one that failed" would not pass by accident. +#[test] +fn test_a_rule_that_walks_a_table_names_the_position_it_stopped_at() { + let hello = include_bytes!("../../test-data/Hello.class"); + + let mut interface = ClassInfo::parse(hello).unwrap(); + let position = interface.interfaces.len() as u16; + interface.interfaces.push("[I".to_string().into()); + assert_eq!( + interface.validate(), + Err(ClassFileError::InvalidFormatAt { + cause: "an interface entry does not name a class", + location: Location::Interface(position), + }) + ); + + let mut method = ClassInfo::parse(hello).unwrap(); + let last = method.methods.len() - 1; + method.methods[last].descriptor = "(V)V".to_string().into(); + assert_eq!( + method.validate(), + Err(ClassFileError::InvalidFormatAt { + cause: "a method descriptor is malformed", + location: Location::Method(last as u16), + }) + ); + + let mut field = ClassInfo::parse(include_bytes!("../../test-data/Field.class")).unwrap(); + let last = field.fields.len() - 1; + field.fields[last].descriptor = "V".to_string().into(); + assert_eq!( + field.validate(), + Err(ClassFileError::InvalidFormatAt { + cause: "a field descriptor is malformed", + location: Location::Field(last as u16), + }) + ); + + // From committed fixtures rather than mutations: the pool index and attribute position are the + // fixture's own, so these pin that the number comes out of the file and not out of the loop. + for (bytes, expected) in [ + ( + &include_bytes!("../../test-data/ldc/LdcDynamicOldMajor.class")[..], + ClassFileError::InvalidFormatAt { + cause: "class file version does not support a constant tag it carries", + location: Location::ConstantPoolEntry(18), + }, + ), + ( + &include_bytes!("../../test-data/ldc/LdcDynamicNoBSM.class")[..], + ClassFileError::InvalidFormatAt { + cause: "a dynamic constant names no bootstrap method", + location: Location::ConstantPoolEntry(11), + }, + ), + ( + &include_bytes!("../../test-data/ldc/LdcDynamicDuplicateBSM.class")[..], + ClassFileError::InvalidFormatAt { + cause: "a single-valued class attribute appears more than once", + location: Location::ClassAttribute(1), + }, + ), + ] { + assert_eq!(ClassInfo::parse(bytes).err(), Some(expected)); + } +} diff --git a/docs/worklog/2026-09-23-validation-rules-name-their-position.json b/docs/worklog/2026-09-23-validation-rules-name-their-position.json new file mode 100644 index 00000000..4ce5c2cc --- /dev/null +++ b/docs/worklog/2026-09-23-validation-rules-name-their-position.json @@ -0,0 +1,23 @@ +{ + "date": "2026-09-23", + "taskId": "rustjava-2026-09-18-bootstrap-argument-index-and-tag-adopt-p0", + "summary": "Counted validate_class's rules (14, not 15; 13 returned a fixed sentence). 11 of the 13 walk a table and knew where they stopped; all 11 now say it, through one new variant whose index type is the table (5 kinds), not the rule (11).", + "changes": [ + "classfile/src/error.rs: ClassFileError::InvalidFormatAt { cause, location } + Location { ConstantPoolEntry, Interface, Field, Method, ClassAttribute }(u16) with Display", + "classfile/src/validation.rs: 11 rules return the first offending position instead of bool; predicates unchanged", + "jvm-bytecode/src/error.rs: InvalidClassFileAt carried through + located_message; src/runtime.rs, test-utils: one ClassFormatError arm each" + ], + "verification": "cargo test --all green; mutants M1-M3 red; pool indices cross-checked with an independent byte walker", + "adoptedProposals": ["2026-09-18-bootstrap-argument-index-and-tag#p0"], + "proposals": [ + { + "title": "Give the three parse-level refusals in class.rs the same treatment only if one of them turns out to hold a position", + "plainSummary": "Three more rejection messages live outside validation (truncated file, extra bytes, version before 45.0); none of them was counted here.", + "userBenefit": "If one of them does know where it stopped, a user reading 'truncated or unparsable' could be told at which byte instead of opening a hex editor.", + "why": "This round counted validate_class only, as the ticket scoped it. 'truncated or unparsable' comes from nom's error, which does carry the remaining input, so an offset may be recoverable; the other two have nothing to point at.", + "tradeoff": "nom's offset is into the parser's view, not a table index, so it would need a sixth Location kind (a byte offset) with a different meaning from the other five. It may well come back as 'not worth it' — which is a fine answer.", + "effort": "S", + "target": "classfile/src/class.rs, classfile/src/error.rs" + } + ] +} diff --git a/docs/worklog/2026-09-23-validation-rules-name-their-position.md b/docs/worklog/2026-09-23-validation-rules-name-their-position.md new file mode 100644 index 00000000..f89a8d1c --- /dev/null +++ b/docs/worklog/2026-09-23-validation-rules-name-their-position.md @@ -0,0 +1,41 @@ +# 2026-09-23 — validation rules name the position they stopped at + +Ticket: `rustjava-2026-09-18-bootstrap-argument-index-and-tag-adopt-p0` · adopts `2026-09-18-bootstrap-argument-index-and-tag#p0`. + +## Count (@origin/main `c654ae2e`) +The premise "15 rules, 14 bool" was off by one: `validate_class` has **14** rules, **13** returning a fixed sentence. + +| # | rule | position in hand | asserted before | +|---|---|---|---| +| 1 | this_class does not name a class | N (one name) | 1 | +| 2 | super_class does not name a class | N (one name) | 0 | +| 3 | an interface entry does not name a class | Y interface index | 0 | +| 4 | a constant pool entry names a missing or wrong-kind entry | Y pool index | 1 | +| 5 | class file version does not support a constant tag | Y pool index | 1 | +| 6 | a dynamic constant names no bootstrap method | Y pool index | 0 | +| 7 | a single-valued class attribute appears more than once | Y attribute position (already `enumerate`d) | 1 | +| 8 | a field descriptor is malformed | Y field index | 0 | +| 9 | multiple ConstantValue attributes on a field | Y field index | 0 | +| 10 | a ConstantValue does not match its field descriptor | Y field index | 0 | +| 11 | a method descriptor is malformed | Y method index | 1 | +| 12 | an abstract or native method carries a Code attribute | Y method index | 0 | +| 13 | a method does not have exactly one Code attribute | Y method index | 1 | + +(bootstrap-argument rule = the 14th, already structured by #73.) + +## Where to stop +Stop criterion: **the enum grows by table kind, never by rule.** 11 rules index into only 5 tables, +so one `InvalidFormatAt { cause, location: Location }` carries all of them and the rule stays the +sentence it already was. A rule qualifies if it walks a table and the failing element's index is a +number a reader can find in the file. Rules 1–2 check one name and have nothing to point at — they +stay `InvalidFormat`. `ClassFileError` stays `Copy` and the same size (the existing size test passes unchanged). + +Converted: **11**. Accept/reject boundary: predicates unchanged, only `all`/`any` → "first offender". + +## Message +` ( #)` — old sentence first so substring asserts and readers still match. +Pool index is the 1-based `#N` `javap -v` prints; other tables are zero-based like `InvalidBootstrapArgument`. +No file bytes (names, descriptors) are carried. + +## Follow-up +- p0: the three parse-level refusals in `class.rs` were out of scope and are not counted. diff --git a/jvm-bytecode/src/error.rs b/jvm-bytecode/src/error.rs index a15d7cc7..c01a4dab 100644 --- a/jvm-bytecode/src/error.rs +++ b/jvm-bytecode/src/error.rs @@ -1,6 +1,6 @@ use alloc::{format, string::String}; -use classfile::ClassFileError; +use classfile::{ClassFileError, Location}; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum ClassDefinitionError { @@ -15,6 +15,11 @@ pub enum ClassDefinitionError { argument_index: u16, actual: Option<&'static str>, }, + /// The classfile-layer `InvalidFormatAt`, carried through for the same reason as the variant above. + InvalidClassFileAt { + cause: &'static str, + location: Location, + }, UnsupportedClassVersion(u16), Verification, UnsupportedFeature(&'static str), @@ -37,6 +42,15 @@ impl ClassDefinitionError { } } +impl ClassDefinitionError { + /// The `ClassFormatError` message for `InvalidClassFileAt` — here for the same two-boundary + /// reason as `bootstrap_argument_message`. The rule's sentence comes first, unchanged, so a + /// reader who knew the old message still finds it; the position follows. + pub fn located_message(cause: &'static str, location: Location) -> String { + format!("{cause} ({location})") + } +} + impl From for ClassDefinitionError { fn from(error: ClassFileError) -> Self { match error { @@ -50,6 +64,7 @@ impl From for ClassDefinitionError { argument_index, actual, }, + ClassFileError::InvalidFormatAt { cause, location } => Self::InvalidClassFileAt { cause, location }, ClassFileError::UnsupportedVersion(version) => Self::UnsupportedClassVersion(version), } } diff --git a/src/runtime.rs b/src/runtime.rs index d68d68e8..5831651a 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -197,6 +197,9 @@ where &ClassDefinitionError::bootstrap_argument_message(method_index, argument_index, actual), ) .await), + Err(ClassDefinitionError::InvalidClassFileAt { cause, location }) => Err(jvm + .exception("java/lang/ClassFormatError", &ClassDefinitionError::located_message(cause, location)) + .await), Err(ClassDefinitionError::UnsupportedClassVersion(version)) => Err(jvm .exception( "java/lang/UnsupportedClassVersionError", diff --git a/test-utils/src/lib.rs b/test-utils/src/lib.rs index 5e7dc7e3..10993310 100644 --- a/test-utils/src/lib.rs +++ b/test-utils/src/lib.rs @@ -342,6 +342,9 @@ impl Runtime for TestRuntime { &ClassDefinitionError::bootstrap_argument_message(method_index, argument_index, actual), ) .await), + Err(ClassDefinitionError::InvalidClassFileAt { cause, location }) => Err(jvm + .exception("java/lang/ClassFormatError", &ClassDefinitionError::located_message(cause, location)) + .await), Err(ClassDefinitionError::UnsupportedClassVersion(version)) => Err(jvm .exception( "java/lang/UnsupportedClassVersionError", diff --git a/tests/test_class_format.rs b/tests/test_class_format.rs index 69737724..ef6ea332 100644 --- a/tests/test_class_format.rs +++ b/tests/test_class_format.rs @@ -631,12 +631,19 @@ async fn test_a_rejected_class_says_why() { ( "test-data/ldc/LdcDynamicDuplicateBSM.class", "./test-data/ldc/", - "a single-valued class attribute appears more than once", + // The position is the fixture's second BootstrapMethods table — the one that duplicates. + "a single-valued class attribute appears more than once (class attribute #1)", ), ( "test-data/ldc/LdcDynamicOldMajor.class", "./test-data/ldc/", - "class file version does not support a constant tag it carries", + // #18 is the fixture's Dynamic entry at major 52; its MethodHandle (#14) is legal there. + "class file version does not support a constant tag it carries (constant pool entry #18)", + ), + ( + "test-data/ldc/LdcDynamicNoBSM.class", + "./test-data/ldc/", + "a dynamic constant names no bootstrap method (constant pool entry #11)", ), ( "test-data/ldc/LdcDynamicBSMArgPastEnd.class",