Modify

Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#8566 closed defect (fixed)

Improper handling of percent-encoded urls towards Overpass-API

Reported by: tyr_asd Owned by: team
Priority: normal Milestone: 14.01
Component: Core Version: tested
Keywords: overpass api Cc: roland.olbricht

Description (last modified by tyr_asd)

When using the import command of the RemoteControl protocol, JOSM doesn't properly handle already percent-encoded URLs: (This is a regression over what JOSM did a few weeks ago, when I last checked.)

Lets say, I want to load the following URL:

http://www.overpass-api.de/api/interpreter?data=node%5B%22foo%22%5D%3Bout%20meta%3B

Using this URL as the parameter "url" of the import command means that I have to url-encode the whole thing (because "url" is a HTTP-GET parameter):

http://localhost:8111/
GET /import?url=http%3A%2F%2Fwww.overpass-api.de%2Fapi%2Finterpreter%3Fdata%3Dnode%255B%2522foo%2522%255D%253Bout%2520meta%253B

but instead of grabbing the original URL, JOSM does this:

GET http://www.overpass-api.de/api/interpreter?data=node%255B%2522foo%2522%255D%253Bout%2520meta%253B

This looks like JOSM is either doing one superfluous url-encoding step or is missing one internal url-decoding step. I found out that after bug #8148 the code was altered slightly - maybe that could be the cause of this?

Attachments (0)

Change History (13)

comment:1 Changed 7 years ago by Don-vip

Cc: roland.olbricht added
Keywords: overpass api added

r5782 (#8505) may be the cause of this behaviour.

comment:2 Changed 7 years ago by tyr_asd

Description: modified (diff)

comment:3 Changed 7 years ago by tyr_asd

Component: Core remotecontrolCore
Summary: RemoteControl: import command doesn't properly handle percent-encoded urlsImproper handling of percent-encoded urls towards Overpass-API

comment:4 Changed 7 years ago by tyr_asd

OK, confirming that the additional URLEncoder.encode() added in r5782 is actually the bad boy.
If you really want to use such a workaround to circumvent charset problems, you'd at least have to decode the url first.

comment:5 Changed 7 years ago by roland.olbricht

Unfortunately, "loadUrl" is called quite straight from the "Open Location ..." dialogue. And people may input there URLs that would break on decoding, e.g. as part of the URL a regular expression that contains a literal plus sign.

Removing the encoding code won't help either, because the JRE behaves when calling URLs platform-dependend and breaks unencoded queries on Windows.

I would suggest one of the following solutions, ordered from most prospective to least prospective:

  1. Encode only if the URL contains a non-ASCII character. This lets pass URLs like the one above but would work around the platform dependend encoding of the JRE. False positives are URLs with only ASCII special characters. However, the Overpass server handles them properly. Thus the main risk would be an accidential encoding by the JRE. Such a behaviour was not observed yet.
  1. Encode only if "loadURL" is called from the download dialogue. I'm not sure whether this is possible in the code with reasonable effort.
  1. Decode URLs if they are valid in percent encoding, i.e. if the percent sign is the only special character (ASCII or non-ASCII) that is contained in the URL. This may affect regularly entered URLs by surprise, so I strongly suggest not to do this. In particular, this may cause a silent rewriting of regular expressions, and that ultimately ends in very hard to debug misbehaviour of that query.

comment:6 Changed 6 years ago by tyr_asd

This is causing troubles, again: https://github.com/tyrasd/overpass-ide/issues/31 :(

Here is the problem:

  1. When calling JOSM's remote control, I cannot url-encode the url because of this bug
  2. Because JOSM fails to load url's containing (non url-encoded) newlines, I have to strip those
  3. But Overpass-QL queries can contain single line comments (// …).
  4. When a query contains such a single line comment, I cannot strip newlines because they have syntactical meaning.
  5. Deadlock :(

As JOSM now has its own "Download from Overpass API" tool, I guess nobody still uses the loadURL tool to manually enter a non-escaped overpass query? So, can we – please – revert changeset r5782 now?

Version 2, edited 6 years ago by tyr_asd (previous) (next) (diff)

comment:7 Changed 6 years ago by simon04

Resolution: fixed
Status: newclosed

In 6588/josm:

fix #8566 - Improper handling of percent-encoded urls towards Overpass-API (reverts r5782)

Adds DownloadOsmTask#modifyUrlBeforeLoad to allow subclasses to do anything with a to be loaded URL.

comment:8 Changed 6 years ago by simon04

In 6589/josm:

see #8566 - make URL patterns of DownloadOsmTask visible for subclasses

comment:9 Changed 6 years ago by simon04

Moved hack to mirrored_download plugin: [o30162], [o30163].

comment:10 Changed 6 years ago by Don-vip

Milestone: 14.01

comment:11 Changed 5 years ago by brycenesbitt

This is causing troubles again, I think.
Overpass generates a URL which gets rejected by JOSM:

Cannot open URL
'http://overpass-api.de/api/interpreter?
fixme=JOSM-ticket-8566
&data=%2F*%0AThis%20has%20been%20generated%20b'...
The following download tasks accept the URL patterns shown:

comment:12 Changed 5 years ago by tyr_asd

Here is the corresponding ticket on github: https://github.com/tyrasd/overpass-turbo/issues/158

Any idea how to fix this?

Last edited 5 years ago by tyr_asd (previous) (diff)

comment:13 Changed 5 years ago by simon04

Please do not reuse very old tickets, but open a new one and refer to the other one. This eases to deal with milestone handling …

#11356

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.