Skip to content

Commit f2fb9b0

Browse files
committed
feat(svg): malformed-number errors name the field and value
num()/viewBox()/transform parsing routed through a parseNumber helper that throws '<field> must be a number, got <value>' with the cause chained, instead of leaking the raw JDK 'For input string'. Drops a dead emitLayer param and fixes the describe() truncation off-by-one. Adds negative tests.
1 parent 2a2f54e commit f2fb9b0

2 files changed

Lines changed: 55 additions & 11 deletions

File tree

src/main/java/com/demcha/compose/document/svg/SvgIconReader.java

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,10 @@ private static double[] viewBox(Element svg) {
101101
if (parts.length != 4) {
102102
throw new IllegalArgumentException("viewBox must carry four numbers: '" + viewBox + "'");
103103
}
104-
double minX = Double.parseDouble(parts[0]);
105-
double minY = Double.parseDouble(parts[1]);
106-
double width = Double.parseDouble(parts[2]);
107-
double height = Double.parseDouble(parts[3]);
104+
double minX = parseNumber(parts[0], "viewBox min-x");
105+
double minY = parseNumber(parts[1], "viewBox min-y");
106+
double width = parseNumber(parts[2], "viewBox width");
107+
double height = parseNumber(parts[3], "viewBox height");
108108
requirePositive(width, height, viewBox);
109109
return new double[]{minX, minY, width, height};
110110
}
@@ -113,8 +113,8 @@ private static double[] viewBox(Element svg) {
113113
if (w.isEmpty() || h.isEmpty()) {
114114
throw new IllegalArgumentException("SVG carries neither a viewBox nor width/height attributes");
115115
}
116-
double width = Double.parseDouble(w);
117-
double height = Double.parseDouble(h);
116+
double width = parseNumber(w, "width");
117+
double height = parseNumber(h, "height");
118118
requirePositive(width, height, w + " x " + h);
119119
return new double[]{0, 0, width, height};
120120
}
@@ -159,7 +159,7 @@ private static void walk(Element element, double[] transform, Paint inherited,
159159
};
160160

161161
if (d != null && !d.isBlank()) {
162-
emitLayer(element, name, d, paint, matrix, box, gradients, out);
162+
emitLayer(element, d, paint, matrix, box, gradients, out);
163163
} else if (DROPS_CONTENT.contains(name)) {
164164
// A shape kind we deliberately don't render — count it so the icon
165165
// surfaces one warning per kind instead of silently losing pixels.
@@ -184,7 +184,7 @@ private static void walk(Element element, double[] transform, Paint inherited,
184184
}
185185

186186
/** Builds and appends the layer for a drawable element (curve geometry + paint). */
187-
private static void emitLayer(Element element, String name, String d, Paint paint,
187+
private static void emitLayer(Element element, String d, Paint paint,
188188
double[] matrix, double[] box, Map<String, Element> gradients,
189189
List<SvgIcon.Layer> out) {
190190
boolean strokeVisible = paint.stroke().visible() && paint.strokeWidth() > 0;
@@ -229,7 +229,7 @@ private static String describe(Element element) {
229229
Node attr = attrs.item(i);
230230
String value = attr.getNodeValue();
231231
if (value != null && value.length() > 40) {
232-
value = value.substring(0, 39) + "…";
232+
value = value.substring(0, 40) + "…";
233233
}
234234
sb.append(' ').append(attr.getNodeName()).append("=\"").append(value).append('"');
235235
}
@@ -330,7 +330,22 @@ static DocumentColor color(String value, DocumentColor current) {
330330

331331
private static double num(Element element, String attribute) {
332332
String value = element.getAttribute(attribute).trim();
333-
return value.isEmpty() ? 0.0 : Double.parseDouble(value);
333+
return value.isEmpty() ? 0.0 : parseNumber(value, attribute);
334+
}
335+
336+
/**
337+
* Parses a numeric SVG value, naming the field and the offending input on
338+
* failure instead of leaking the raw {@link NumberFormatException} message
339+
* ("For input string: …"). The cause is chained so the JDK detail survives
340+
* for anyone who needs it.
341+
*/
342+
private static double parseNumber(String value, String what) {
343+
try {
344+
return Double.parseDouble(value);
345+
} catch (NumberFormatException e) {
346+
throw new IllegalArgumentException(
347+
what + " must be a number, got '" + value + "'", e);
348+
}
334349
}
335350

336351
static double[] identity() {
@@ -375,7 +390,7 @@ static double[] compose(double[] parent, String transformAttribute) {
375390
private static double[] transformOp(String op, String[] args, String source) {
376391
double[] v = new double[args.length];
377392
for (int i = 0; i < args.length; i++) {
378-
v[i] = Double.parseDouble(args[i]);
393+
v[i] = parseNumber(args[i], "transform '" + source + "' argument");
379394
}
380395
return switch (op) {
381396
case "translate" -> new double[]{1, 0, 0, 1, v[0], v.length > 1 ? v[1] : 0};

src/test/java/com/demcha/compose/document/svg/SvgIconTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,4 +533,33 @@ void blankIconExplainsWhatItSkipped() {
533533
.hasMessageContaining("skipped text")
534534
.hasMessageContaining("vector shapes only");
535535
}
536+
537+
@Test
538+
void malformedNumericAttributeNamesElementAttributeAndReason() {
539+
// A non-numeric geometry attribute must name the element AND say which
540+
// field expected a number — not leak a bare JDK "For input string".
541+
assertThatThrownBy(() -> SvgIcon.parse("""
542+
<svg viewBox="0 0 10 10">
543+
<rect x="0" y="0" width="abc" height="10" fill="#000"/>
544+
</svg>
545+
"""))
546+
.isInstanceOf(IllegalArgumentException.class)
547+
.hasMessageContaining("<rect")
548+
.hasMessageContaining("width must be a number")
549+
.hasMessageContaining("abc");
550+
}
551+
552+
@Test
553+
void malformedViewBoxNumberSaysWhatExpectedANumber() {
554+
// viewBox is parsed before the element walk, so its own error must be
555+
// self-describing rather than a bare JDK parse message.
556+
assertThatThrownBy(() -> SvgIcon.parse("""
557+
<svg viewBox="0 0 ten 10">
558+
<rect width="10" height="10" fill="#000"/>
559+
</svg>
560+
"""))
561+
.isInstanceOf(IllegalArgumentException.class)
562+
.hasMessageContaining("viewBox")
563+
.hasMessageContaining("must be a number");
564+
}
536565
}

0 commit comments

Comments
 (0)