Modify

Opened 6 years ago

Closed 6 years ago

#9560 closed defect (fixed)

IllegalArgumentException when color.layer contains "{ }"

Reported by: chihchun Owned by: team
Priority: normal Milestone: 14.01
Component: Core Version: latest
Keywords: regression color preferences Cc: bastiK

Description (last modified by Don-vip)

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

Attachments (2)

0001-Fix-9560-only-use-MessageFormat.format-when-needed.patch (981 bytes) - added by chihchun 6 years ago.
9560_v2.patch (2.9 KB) - added by simon04 6 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by chihchun

Please see the patch for fix.

comment:2 Changed 6 years ago by 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.

comment:3 Changed 6 years ago by 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.

comment:4 Changed 6 years ago by Don-vip

Cc: bastiK added
Description: modified (diff)
Keywords: regression color preferences added
Milestone: 14.01

comment:5 in reply to:  3 Changed 6 years ago by 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.

comment:6 Changed 6 years ago by 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.

comment:7 Changed 6 years ago by 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.

comment:8 in reply to:  7 Changed 6 years ago by 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 '.

comment:9 in reply to:  6 Changed 6 years ago by 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 …

Changed 6 years ago by simon04

Attachment: 9560_v2.patch added

comment:10 Changed 6 years ago by stoecker

Don't see an obvious error with that patch.

comment:11 Changed 6 years ago by simon04

Resolution: fixed
Status: newclosed

In 6671/josm:

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.