Pre-loading graphs into the composer using a query string breaks if spaces are in any of the targets

Bug #932537 reported by Michael Leinartas
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Graphite
Fix Released
Medium
Unassigned

Bug Description

I'm noticing a problem with url escaping/encoding through graphite. It has to do with python's url libs encoding spaces as pluses, and Ext's url decoding not decoding pluses to spaces.

If I create a graph with this target (via the composer):
groupByNode(carbon.agents.wfmu_local-a.creates, 1, "sumSeries")

and I then copy out the image url, it works
http://wfmu/render/?width=586&height=308&_salt=1325010004.15&target=groupByNode(carbon.agents.wfmu_local-a.creates%2C%201%2C%20%22sumSeries%22)

but if I then try to go back to the composer, it fails with a broken image/parse
http://wfmu/composer/?width=586&height=308&_salt=1325010004.15&target=groupByNode(carbon.agents.wfmu_local-a.creates%2C%201%2C%20%22sumSeries%22)

the reason is that in composer/views.py, the line: 'queryString' : request.GET.urlencode(),
ends up encoding as target=groupByNode%28carbon.agents.wfmu_local-a.creates%2C+1%2C+%22sumSeries%22%29
(spaces translated to pluses)

which is then set in the JS, and parsed in composer.js:loadURL with Ext.urlDecode, but Ext.urlDecode doesn't decode pluses into spaces, so graphite gets confused about how to parse the expression.

Ext.urlDecode('x=a+a')
Object
x: "a+a"
__proto__: Object

simplest fix seems to be changing composer/views.py:composer to 'queryString' : request.GET.urlencode().replace('+', '%20'),

thanks,
--dave

Revision history for this message
Michael Leinartas (mleinartas) wrote :

Confirmed this. It actually doesn't seem to like it whether it's a %20 or a + for me

Changed in graphite:
importance: Undecided → Medium
milestone: none → 0.9.10
status: New → Confirmed
Revision history for this message
Michael Leinartas (mleinartas) wrote :

I misinterpreted, I see now. The reporter is exactly right. Django's QueryDict.urlencode() is encoding with a + (normal for a querystring) but Ext doesnt decode that as a space - that causes a + to actually show in the target itself (which can be seen if Graph Data is clicked).

Reporter's solution is reasonable and I dont see a better way. Committed in r687

Changed in graphite:
status: Confirmed → Fix Committed
Changed in graphite:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Related questions

Remote bug watches

Bug watches keep track of this bug in other bug trackers.