Skip to content
This repository has been archived by the owner on Jul 24, 2021. It is now read-only.

API 0.6 ChangesetController#upload mishandles XML #1565

Closed
openstreetmap-trac opened this issue Jul 23, 2021 · 2 comments
Closed

API 0.6 ChangesetController#upload mishandles XML #1565

openstreetmap-trac opened this issue Jul 23, 2021 · 2 comments

Comments

@openstreetmap-trac
Copy link

Reporter: ivansanchez[at]escomposlinux.org
[Submitted to the original trac issue database at 11.05pm, Saturday, 7th February 2009]

I'm using the rails port rev 13578 (2009-02-08) and a custom script intended for bulk uploads.

It seems that the changeset controller is not able to correctly handle newlines and whitespace in the XML.

How to reproduce: make a script open a changeset and upload a chunk of XML using POST /api/0.6/changeset/#id/upload, but with no whitespace between XML elements (e.g. use "tidy"). Half of the elements being uploaded will get lost:


Some of my test cases follow. With whitespace between the nodes, the upload will go OK:

Sending payload for changeset 33: "
<osmChange version='0.6' generator='php_bulk_uploader'><create version='0.6' generator='php_bulk_uploader'> <node id="-1" visible="1" lat="43.773532887011129" lon="-7.904814920323187" changeset="33"/> <node id="-2" visible="1" lat="43.773442030212991" lon="-7.904729601316934" changeset="33"/> <node id="-3" visible="1" lat="43.773351769284325" lon="-7.904706400406464" changeset="33"/> <node id="-4" visible="1" lat="43.77326210426007" lon="-7.904745317310963" changeset="33"/> <node id="-5" visible="1" lat="43.773226571935723" lon="-7.9047956699588" changeset="33"/> <node id="-6" visible="1" lat="43.77321864218105" lon="-7.904907646441022" changeset="33"/> <node id="-7" visible="1" lat="43.773246483046684" lon="-7.904994117486803" changeset="33"/> <node id="-8" visible="1" lat="43.773300973187915" lon="-7.905042824227181" changeset="33"/> <node id="-9" visible="1" lat="43.773409357712573" lon="-7.905078120062009" changeset="33"/> <node id="-10" visible="1" lat="43.773490258886241" lon="-7.905064215324585" changeset="33"/> </create></osmChange>
"

IDs updated: "
<?xml version="1.0" encoding="UTF-8"?>
<diffResult version="0.6" generator="OpenStreetMap server">
  <node old_id="-1" new_id="45" new_version="1"/>
  <node old_id="-2" new_id="46" new_version="1"/>
  <node old_id="-3" new_id="47" new_version="1"/>
  <node old_id="-4" new_id="48" new_version="1"/>
  <node old_id="-5" new_id="49" new_version="1"/>
  <node old_id="-6" new_id="50" new_version="1"/>
  <node old_id="-7" new_id="51" new_version="1"/>
  <node old_id="-8" new_id="52" new_version="1"/>
  <node old_id="-9" new_id="53" new_version="1"/>
  <node old_id="-10" new_id="54" new_version="1"/>
</diffResult>
"

With no whitespace before the first , it will get lost:

Sending payload for changeset 32: "
<osmChange version='0.6' generator='php_bulk_uploader'><create version='0.6' generator='php_bulk_uploader'><node id="-1" visible="1" lat="43.773532887011129" lon="-7.904814920323187" changeset="32"/> <node id="-2" visible="1" lat="43.773442030212991" lon="-7.904729601316934" changeset="32"/> <node id="-3" visible="1" lat="43.773351769284325" lon="-7.904706400406464" changeset="32"/> <node id="-4" visible="1" lat="43.77326210426007" lon="-7.904745317310963" changeset="32"/> <node id="-5" visible="1" lat="43.773226571935723" lon="-7.9047956699588" changeset="32"/> <node id="-6" visible="1" lat="43.77321864218105" lon="-7.904907646441022" changeset="32"/> <node id="-7" visible="1" lat="43.773246483046684" lon="-7.904994117486803" changeset="32"/> <node id="-8" visible="1" lat="43.773300973187915" lon="-7.905042824227181" changeset="32"/> <node id="-9" visible="1" lat="43.773409357712573" lon="-7.905078120062009" changeset="32"/> <node id="-10" visible="1" lat="43.773490258886241" lon="-7.905064215324585" changeset="32"/> </create></osmChange>
"

IDs updated: "
<?xml version="1.0" encoding="UTF-8"?>
<diffResult version="0.6" generator="OpenStreetMap server">
  <node old_id="-2" new_id="36" new_version="1"/>
  <node old_id="-3" new_id="37" new_version="1"/>
  <node old_id="-4" new_id="38" new_version="1"/>
  <node old_id="-5" new_id="39" new_version="1"/>
  <node old_id="-6" new_id="40" new_version="1"/>
  <node old_id="-7" new_id="41" new_version="1"/>
  <node old_id="-8" new_id="42" new_version="1"/>
  <node old_id="-9" new_id="43" new_version="1"/>
  <node old_id="-10" new_id="44" new_version="1"/>
</diffResult>
"

With no whitespace at all, half of the nodes will get lost:

Sending payload for changeset 34: "
<osmChange version='0.6' generator='php_bulk_uploader'><create version='0.6' generator='php_bulk_uploader'><node id="-1" visible="1" lat="43.773532887011129" lon="-7.904814920323187" changeset="34"/><node id="-2" visible="1" lat="43.773442030212991" lon="-7.904729601316934" changeset="34"/><node id="-3" visible="1" lat="43.773351769284325" lon="-7.904706400406464" changeset="34"/><node id="-4" visible="1" lat="43.77326210426007" lon="-7.904745317310963" changeset="34"/><node id="-5" visible="1" lat="43.773226571935723" lon="-7.9047956699588" changeset="34"/><node id="-6" visible="1" lat="43.77321864218105" lon="-7.904907646441022" changeset="34"/><node id="-7" visible="1" lat="43.773246483046684" lon="-7.904994117486803" changeset="34"/><node id="-8" visible="1" lat="43.773300973187915" lon="-7.905042824227181" changeset="34"/><node id="-9" visible="1" lat="43.773409357712573" lon="-7.905078120062009" changeset="34"/><node id="-10" visible="1" lat="43.773490258886241" lon="-7.905064215324585" changeset="34"/></create></osmChange>
"

IDs updated: "
<?xml version="1.0" encoding="UTF-8"?>
<diffResult version="0.6" generator="OpenStreetMap server">
  <node old_id="-2" new_id="55" new_version="1"/>
  <node old_id="-4" new_id="56" new_version="1"/>
  <node old_id="-6" new_id="57" new_version="1"/>
  <node old_id="-8" new_id="58" new_version="1"/>
  <node old_id="-10" new_id="59" new_version="1"/>
</diffResult>
"

If there is whitespace between and , then the odd nodes will get uploaded and the even ones will get lost...

Sending payload for changeset 31: "
<osmChange version='0.6' generator='php_bulk_uploader'>
<create version='0.6' generator='php_bulk_uploader'>
<node id="-1" visible="1" lat="43.773532887011129" lon="-7.904814920323187" changeset="31"/><node id="-2" visible="1" lat="43.773442030212991" lon="-7.904729601316934" changeset="31"/><node id="-3" visible="1" lat="43.773351769284325" lon="-7.904706400406464" changeset="31"/><node id="-4" visible="1" lat="43.77326210426007" lon="-7.904745317310963" changeset="31"/><node id="-5" visible="1" lat="43.773226571935723" lon="-7.9047956699588" changeset="31"/><node id="-6" visible="1" lat="43.77321864218105" lon="-7.904907646441022" changeset="31"/><node id="-7" visible="1" lat="43.773246483046684" lon="-7.904994117486803" changeset="31"/><node id="-8" visible="1" lat="43.773300973187915" lon="-7.905042824227181" changeset="31"/><node id="-9" visible="1" lat="43.773409357712573" lon="-7.905078120062009" changeset="31"/><node id="-10" visible="1" lat="43.773490258886241" lon="-7.905064215324585" changeset="31"/></create></osmChange>
"

IDs updated: "
<?xml version="1.0" encoding="UTF-8"?>
<diffResult version="0.6" generator="OpenStreetMap server">
  <node old_id="-1" new_id="31" new_version="1"/>
  <node old_id="-3" new_id="32" new_version="1"/>
  <node old_id="-5" new_id="33" new_version="1"/>
  <node old_id="-7" new_id="34" new_version="1"/>
  <node old_id="-9" new_id="35" new_version="1"/>
</diffResult>
"

Also, sending a will raise an error:

Sending payload for changeset 36: "
<osmChange version='0.6' generator='php_bulk_uploader'><create version='0.6' generator='php_bulk_uploader'>
<node lat='0' lon='0 changeset='36'></node>
</create></osmChange>
"

<h1>
  LibXML::XML::Error
    in ChangesetController#upload
</h1>
<pre>Fatal error: attributes construct error at :2.</pre>

Exactly the same error is triggered when the node has tags, with or without whitespace:

<osmChange version='0.6' generator='php_bulk_uploader'><create version='0.6' generator='php_bulk_uploader'>
<node lat='0' lon='0 changeset='37'><tag k='foo' v='bar' /></node>
</create></osmChange>
<osmChange version='0.6' generator='php_bulk_uploader'><create version='0.6' generator='php_bulk_uploader'>
<node lat='0' lon='0 changeset='38'> <tag k='foo' v='bar' /> </node>
</create></osmChange>

I'm not proficient in Ruby, so I cannot track down the bug.

@openstreetmap-trac
Copy link
Author

Author: ivansanchez[at]escomposlinux.org
[Added to the original trac issue at 2.46am, Sunday, 8th February 2009]

Sorry, the last three test cases are completely wrong (the longitude quotes are not closed).

However,

Sending payload for changeset 56: "
<osmChange version='0.6' generator='php_bulk_uploader'><create version='0.6' generator='php_bulk_uploader'>
<node id='-1' lat='0' lon='0' changeset='56' /><node id='-2' lat='0' lon='0' changeset='56'></node>
<node id='-3' lat='0' lon='0' changeset='56' />
</create></osmChange>
"

IDs updated: "
Unknown action node, choices are create, modify, delete.
"
Sending payload for changeset 57: "
<osmChange version='0.6' generator='php_bulk_uploader'><create version='0.6' generator='php_bulk_uploader'>
<node id='-1' lat='0' lon='0' changeset='57' /><node id='-2' lat='0' lon='0' changeset='57'></node>
</create></osmChange>
"

IDs updated: "
<?xml version="1.0" encoding="UTF-8"?>
<diffResult version="0.6" generator="OpenStreetMap server">
  <node old_id="-1" new_id="131" new_version="1"/>
</diffResult>
"

@openstreetmap-trac
Copy link
Author

Author: Matt
[Added to the original trac issue at 8.35am, Sunday, 8th February 2009]

Fixed, I think, in r13579. Looks like the diff reader was skipping elements when it shouldn't have been. I've added a test case similar to yours to the standard test suite.

Cheers for finding this bug!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant