IllegalArgumentException when color.layer contains "{ }"

[chihchun]

When having this config in preferences.xml, it caused josm throws IllegalArgumentException
<tag key='color.layer {5DE308C0-916F-4B5A-B3DB-D45E17F30172}.gpx' value='#FF0000'/>

Because org.openstreetmap.josm.data.Preferences.getColorName(Preferences.java:746) just passed the layer name as key to org.openstreetmap.josm.tools.I18n.tr(I18n.java:157).

The braces need to be escaped.

WARNING: java.lang.IllegalArgumentException: can't parse argument number: 5DE308C0-916F-4B5A-B3DB-D45E17F30172. Cause: java.lang.NumberFormatException: For input string: "5DE308C0-916F-4B5A-B3DB-D45E17F30172"
java.lang.IllegalArgumentException: can't parse argument number: 5DE308C0-916F-4B5A-B3DB-D45E17F30172
	at java.text.MessageFormat.makeFormat(MessageFormat.java:1420)
	at java.text.MessageFormat.applyPattern(MessageFormat.java:479)
	at java.text.MessageFormat.<init>(MessageFormat.java:363)
	at java.text.MessageFormat.format(MessageFormat.java:835)
	at org.openstreetmap.josm.tools.I18n.tr(I18n.java:157)
	at org.openstreetmap.josm.data.Preferences.getColorName(Preferences.java:746)
	at org.openstreetmap.josm.gui.preferences.display.ColorPreference.getName(ColorPreference.java:142)
	at org.openstreetmap.josm.gui.preferences.display.ColorPreference.setColorModel(ColorPreference.java:93)
	at org.openstreetmap.josm.gui.preferences.display.ColorPreference.addGui(ColorPreference.java:148)
	at org.openstreetmap.josm.gui.preferences.PreferenceTabbedPane.stateChanged(PreferenceTabbedPane.java:511)
	at javax.swing.DefaultSingleSelectionModel.fireStateChanged(DefaultSingleSelectionModel.java:132)
	at javax.swing.DefaultSingleSelectionModel.setSelectedIndex(DefaultSingleSelectionModel.java:67)
	at javax.swing.JTabbedPane.setSelectedIndexImpl(JTabbedPane.java:616)
	at javax.swing.JTabbedPane.setSelectedIndex(JTabbedPane.java:591)
	at javax.swing.JTabbedPane.insertTab(JTabbedPane.java:731)
	at javax.swing.JTabbedPane.addTab(JTabbedPane.java:767)
	at org.openstreetmap.josm.gui.preferences.PreferenceTabbedPane.addGUITabs(PreferenceTabbedPane.java:410)
	at org.openstreetmap.josm.gui.preferences.PreferenceTabbedPane.buildGui(PreferenceTabbedPane.java:367)
	at org.openstreetmap.josm.gui.preferences.PreferenceDialog.build(PreferenceDialog.java:69)
	at org.openstreetmap.josm.gui.preferences.PreferenceDialog.<init>(PreferenceDialog.java:82)
	at org.openstreetmap.josm.actions.PreferencesAction.run(PreferencesAction.java:66)
	at org.openstreetmap.josm.actions.PreferencesAction.actionPerformed(PreferencesAction.java:61)
	at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:2018)
	at javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2341)
	at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:402)
	at javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:259)
	at javax.swing.AbstractButton.doClick(AbstractButton.java:376)
	at javax.swing.plaf.basic.BasicMenuItemUI.doClick(BasicMenuItemUI.java:833)
	at javax.swing.plaf.basic.BasicMenuItemUI$Handler.mouseReleased(BasicMenuItemUI.java:877)
	at java.awt.AWTEventMulticaster.mouseReleased(AWTEventMulticaster.java:289)
	at java.awt.Component.processMouseEvent(Component.java:6505)
	at javax.swing.JComponent.processMouseEvent(JComponent.java:3312)
	at java.awt.Component.processEvent(Component.java:6270)
	at java.awt.Container.processEvent(Container.java:2229)
	at java.awt.Component.dispatchEventImpl(Component.java:4861)
	at java.awt.Container.dispatchEventImpl(Container.java:2287)
	at java.awt.Component.dispatchEvent(Component.java:4687)
	at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4832)
	at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4492)
	at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4422)
	at java.awt.Container.dispatchEventImpl(Container.java:2273)
	at java.awt.Window.dispatchEventImpl(Window.java:2719)
	at java.awt.Component.dispatchEvent(Component.java:4687)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:735)
	at java.awt.EventQueue.access$200(EventQueue.java:103)
	at java.awt.EventQueue$3.run(EventQueue.java:694)
	at java.awt.EventQueue$3.run(EventQueue.java:692)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:76)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:87)
	at java.awt.EventQueue$4.run(EventQueue.java:708)
	at java.awt.EventQueue$4.run(EventQueue.java:706)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:76)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:705)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:242)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:161)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:150)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:146)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:138)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:91)
Caused by: java.lang.NumberFormatException: For input string: "5DE308C0-916F-4B5A-B3DB-D45E17F30172"
	at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
	at java.lang.Integer.parseInt(Integer.java:492)
	at java.lang.Integer.parseInt(Integer.java:527)
	at java.text.MessageFormat.makeFormat(MessageFormat.java:1418)
	... 60 more

[chihchun]

Please see the patch for fix.

[stoecker]

You fix may work, but is at the wrong place. Correct would be to change the getColor() in Preferences, so it escapes the illegal signs like it does for some others.

[chihchun]

Hi, stoecker

I was thinking about escape the name in getColor(), the user could see the color name with escape chars.

This patch fixed the tr function which should not the makeFormat when it's not necessary.

[stoecker]

Replying to chihchun <rex.cc.tsai@…>:

Hi, stoecker

I was thinking about escape the name in getColor(), the user could see the color name with escape chars.

This patch fixed the tr function which should not the makeFormat when it's not necessary.

Checked again, your patch breaks other places. We always, in all texts use proper escaping. Otherwise translation gets awful. Also my idea is wrong, as this is already done. The "{}" must be escaped with ' for a proper solution.

[stoecker]

Best would be an function in utils or i18n which does escape properly and a call to this function in the layer-context, i.e. where the layer name is passed to getColor(). The bad entry in your prefs should be removed by hand.

[chihchun]

No, the patch should not break other places. It simply ignore empty object array passed by other function. Ex: tr('translate') which send an empty array.

[stoecker]

Replying to chihchun <rex.cc.tsai@…>:

No, the patch should not break other places. It simply ignore empty object array passed by other function. Ex: tr('translate') which send an empty array.

It skips the escaping - So '' will remain instead of getting a single '.

[simon04]

Replying to stoecker:

Best would be an function in utils or i18n which does escape properly and a call to this function in the layer-context, i.e. where the layer name is passed to getColor(). The bad entry in your prefs should be removed by hand.

We already have I18n.escape. Patch on the way …

[stoecker]

Don't see an obvious error with that patch.

[simon04]

In 6671/josm:

fix #9560 - IllegalArgumentException when color.layer contains "{ }"