[Patch] Empty parentheses in search string lead to early termination of expression parsing

[Woazboat]

What steps will reproduce the problem?

1. Open the search dialog
2. Search for e.g. foo () bar

What is the expected result?

Search expressions with any number of empty parenthesis pair at the top level such as foo () bar should be considered valid.

What happens instead?

() in a search expression at the top level leads to incorrect early termination of expression parsing.

Please provide any additional information below. Attach a screenshot if possible.

The JOSM search expression parser in SearchCompiler.java -> parseExpression(), repeatedly calls parseFactor() until it returns null (which usually signifies EOF/end of string). Unfortunately parseFactor() also returns null when it encounters an empty parenthesis pair (), leading to early termination of expression parsing. E.g. foo () is considered a valid search expression while foo () bar is not.

(Note that e.g. parent () foo works because the returned null from parsing () is passed to the constructor of the Parent() match object, which treats null as matching everything.)

Revision:18289
Is-Local-Build:true
Build-Date:2021-10-22 22:18:47

Identification: JOSM/1.5 (18289 SVN en_GB) Linux Debian GNU/Linux bookworm/sid
Memory Usage: 408 MB / 3982 MB (150 MB allocated, but free)
Java version: 12.0.2+9-Debian-1, Debian, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Screen: :0.0 1920×1080 (scaling 1.00×1.00) :0.1 1920×1080 (scaling 1.00×1.00) :0.2 1920×1080 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→16×16, 32×32→32×32
Environment variable LANG: en_AT.UTF-8
System property file.encoding: ANSI_X3.4-1968
System property sun.jnu.encoding: ANSI_X3.4-1968
Locale info: en_GB
Numbers with default locale: 1234567890 -> 1234567890
Desktop environment: KDE
Java ATK Wrapper package: libatk-wrapper-java:all-0.38.0-5
libcommons-compress-java: libcommons-compress-java:all-1.21-1
libcommons-logging-java: libcommons-logging-java:all-1.2-2
fonts-noto: fonts-noto:all-20201225-1
liboauth-signpost-java: liboauth-signpost-java:all-1.2.1.2-3
Dataset consistency test: No problems found

Plugins:
+ CADTools (1008)
+ DirectDownload (35640)
+ EasyPresets (1623509627)
+ FastDraw (35640)
+ Lanes (${version.entry.commit.revision})
+ Mapillary (2.0.0-alpha.36-dirty)
+ OpeningHoursEditor (35640)
+ PicLayer (1.0.1)
+ alignways (35753)
+ apache-commons (35524)
+ apache-http (35589)
+ areaselector (368)
+ austriaaddresshelper (1597341117)
+ buildings_tools (35823)
+ centernode (v1.0.4)
+ changeset-viewer (25)
+ conflation (0.6.6)
+ contourmerge (v0.1.8)
+ editgpx (35562)
+ ejml (35458)
+ flatlaf (35799)
+ geotools (35458)
+ gridify (1606242219)
+ gson (35458)
+ imagery_offset_db (35640)
+ indoorhelper (1.2.0)
+ intersection (0.0.7)
+ jaxb (35543)
+ jna (35662)
+ jogl (1.2.3)
+ jts (35458)
+ kartverketimport (35)
+ kendzi3d (1.0.205)
+ kendzi3d-resources (0.0.2)
+ log4j (35458)
+ measurement (35640)
+ opendata (35803)
+ openqa (0.2.2)
+ pt_assistant (1ff2e15)
+ reltoolbox (35829)
+ reverter (35732)
+ rex (53)
+ shrinkwrap (v1.0.4)
+ splinex (35718)
+ tageditor (35640)
+ terracer (35640)
+ todo (30306)
+ turnlanes-tagging (288)
+ turnrestrictions (35640)
+ utilsplugin2 (35792)
+ wikipedia (1.1.4)

Tagging presets:
+ https://github.com/kendzi/Simple3dBuildingsPreset/releases/download/0.9_2018-05-08/s3db-preset.zip
+ https://josm.openstreetmap.de/josmfile?page=Presets/BicycleJunction&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Historic_Stone&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Bus_lanes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Healthcare&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Heritage&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Historical_Objects&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Industrial&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Leaftype&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Light_sources&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Manholes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/MastAndTower&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/public_bookcase&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Simple_Indoor_Tagging&zip=1
+ https://raw.githubusercontent.com/species/josm-preset-wheelchair/master/sidewalks_kerbs.xml
+ https://osmtools.de/josm/steps.xml
+ https://josm.openstreetmap.de/josmfile?page=Presets/StreetCabinet&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Surveillance&zip=1
+ https://raw.githubusercontent.com/yopaseopor/traffic_signs_preset_JOSM/master/A.zip
+ https://josm.openstreetmap.de/josmfile?page=Presets/Trees&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/COVID-19&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/LaneAttributes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/ParkingLanes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/PublicTransportOneClick&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/TurnLanes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/contact(socialnetworks_IMs)&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Playground_Equipment&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/PublicTransportGtfs&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Windrad&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/AdvertisingPreset&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Community_Centre&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Crafts&zip=1
+ https://gitlab.com/k127/josm-presets/raw/master/diplomatic.xml
+ https://josm.openstreetmap.de/josmfile?page=Presets/Mountains&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/NewTags&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Disc_Golf_Course&zip=1
+ ${HOME}/josm/josm_area_highway_preset.xml
+ <josm.userdata>/EasyPresets.xml
+ ${HOME}/syncthing/Sync/Projects/OSM/josm/core/resources/data/defaultpresets.xml

Map paint styles:
+ ${HOME}/syncthing/Sync/Projects/OSM/josm_git/resources/styles/standard/elemstyles.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/PublicTransport&zip=1
+ https://raw.githubusercontent.com/yopaseopor/indoormap/master/indoormap-style.mapcss
+ https://josm.openstreetmap.de/josmfile?page=Styles/Bench&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Streets&zip=1
+ https://raw.githubusercontent.com/species/josm-preset-traffic_sign_direction/master/direction.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/Enhanced_Lane_and_Road_Attributes&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1
+ https://github.com/osmlab/appledata/archive/josm_paint_inline_validation.zip
- https://josm.openstreetmap.de/josmfile?page=Styles/Maxspeed&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Modified&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/plan.at&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/PublicTransportV2&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Osmc&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/sac_scale&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Sidewalks&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/ParkingLanes&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/BesideTheRoad_Speed&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/AdvertisingStyle&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Admin_Boundaries&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Building_Levels_Labels&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Kerbs&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Postcode&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Suburb&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/DestinationSignRelation&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Direction&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/FixmeAndNote&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Landcover&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Mountains&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/NoFeature&zip=1
- https://raw.githubusercontent.com/OpenSidewalks/OpenSidewalks-Schema/master/open_sidewalks.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/PTStops&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/RecyclingMaterials&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/SimpleBuildingTags&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/SimpleRoofTags&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Noname&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Surface&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Ph_Typhoon&zip=1
- https://github.com/igitov/forest-josm-style/archive/master.zip
- https://josm.openstreetmap.de/josmfile?page=Styles/Incline&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/LayerChecker&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/LitObjects&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Power&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/PriorityRoad&zip=1
- ${HOME}/josm/josm_area_highway_style.mapcss
+ https://josm.openstreetmap.de/josmfile?page=Styles/SidewalksAndFootways&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Direction&zip=1
- <josm.userdata>/styles/sit.mapcss

Validator rules:
+ https://josm.openstreetmap.de/josmfile?page=Rules/OSMLint&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Rules/IndoorRules&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Rules/QAToolInspiredValidations&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Rules/KeepRight&zip=1
- https://josm.openstreetmap.de/josmfile?page=Rules/PublicTransportGtfs&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Rules/Pictures&zip=1
+ ${HOME}/josm/josm_area_highway_validator.validator.mapcss
+ ${HOME}/josm/heritage_bda.validator.mapcss

Last errors/warnings:
- 00006.466 W: Update plug-ins - You updated your JOSM software. To prevent problems the plug-ins should be updated as well.  Update plug-ins now?
- 00012.007 W: Not a single layer for the name 'Basiskarte Graz (2015) - 0': []
- 00013.868 E: Failed to locate image 'presets/service/embassy.svg'
- 00013.869 W:  null: Could not get presets icon presets/service/embassy.svg
- 00014.565 W: Cannot lock cache directory. Will not use disk cache
- 00016.724 E: Failed to locate image 'bus.png'
- 00020.449 W: Cannot start IPv4 remotecontrol server on port 8111: Address already in use (Bind failed)
- 00020.450 W: Cannot start IPv6 remotecontrol server on port 8111: Address already in use (Bind failed)

[Woazboat]

Changes/patch can also be found here: ​https://github.com/JOSM/josm/pull/76

​https://github.com/JOSM/josm/pull/76.patch

[skyper]

final SearchCompiler.Match c = SearchCompiler.compile("foo () () () bar");
assertTrue(c.match(OsmUtils.createPrimitive("node name=bar")));

This looks strange. I think both expressions need to be present:

final SearchCompiler.Match c = SearchCompiler.compile("foo () () () bar");
assertTrue(c.match(OsmUtils.createPrimitive("node foo=bar")));
assertFalse(c.match(OsmUtils.createPrimitive("node name=bar")));

[Woazboat]

Ah, yes you're right. That test was something I quickly slapped together afterwards and I (still) have no means to execute unit tests locally. (Because the ant package in debian is missing some jar files)

[Woazboat]

Simple patch that should fix this issue by returning the Always matcher instead of null for empty parentheses

[Woazboat]

I updated the patch

[Don-vip]

In 18291/josm:

fix #21463 - Fix parsing of empty parenthesis pairs in search expressions (patch by Woazboat)

[Don-vip]

Thanks!