| I've noticed this bug while testing mal_tags script. For some reason, when user submits custom tags, decodeURIComponent() (or something with the same effect) is used on each submitted tag to generate both - the tag href and the tag text. This is completely out of logic. In result of this: 1. Users are limited with chars to use in tags, because some of them are reserved for special purposes in hrefs, and they are not encoded here (which is opposite of what is going on). 2. Users cannot use some char sequences (those which represent special characters), because those sequences are decoded into respective characters and the user doesn't get the desired result. If you don't get what i'm talking about, please visit this page: http://www.javascripter.net/faq/escape.htm Steps to reproduce: 1. Go to your animelist or mangalist. 2. Press "edit" button in the "Tags" column. 3. Enter "&" or "#". 4. Press "Save". 5. Press "edit" button again. 6. Enter one of the special codes from the right column of this list: http://www.javascripter.net/faq/escape.htm#upper_ascii 7. Press "Save". As you can see, this is very stupid. The correct way to do this is: - Use encodeURIComponent() to generate the tag href. - Don't use any processing to generate the tag text. This way: - if user submits "&" or "#", the tag href will contain "%26" or "%23", and the tag text will contain "&" or "#", accordingly. - if user submits, for example "%25", then the tag href will contain "%2525", and the tag text will contain "%25". - win! |
 |
| Just to be clear, these entities are fine if tags are added from the advanced edit window for each series, just not from the lists. It's only the javascript on the list pages at fault here. If the tags are used properly there really shouldn't be a high chance of hitting these characters anyway, but if the user does come across them then tag lists can be truncated or otherwise broken.... so it should be fixed. The tag text should not be url-escaped on submit, because it's the browser's responsibility to do this internally when formatting the POST data. |
 I am a banana. |
saka said: Yes, the tag text is generated correctly (exactly as user inputed) if user submits it from the advanced edit window. But the problem is, that the tag is not encoded to generate the tag href, even from advanced edit window. So if the user submits, for example a tag named "boobs&guns", from advanced edit window, the tag text will be as user desired, but the tag href will be Just to be clear, these entities are fine if tags are added from the advanced edit window for each series, just not from the lists. It's only the javascript on the list pages at fault here. http://myanimelist.net/animelist/username&tag=boobs&guns And the same problem goes for escape codes: if user submits "%26" from the advanced edit window, the tag text will be "%26" (as desired), but the tag href will be http://myanimelist.net/animelist/username&tag=%26 So as i said - submitted tags must each be encoded first to generate tag hrefs. saka said: I understand this.The tag text should not be url-escaped on submit, because it's the browser's responsibility to do this internally when formatting the POST data. But you don't get the point here. The tag should ultimately result in exactly what user entered. In a case when forms do not use «enctype="multipart/form-data"», if it is an escape code, then it will be encoded once more by the browser on submit, and the server will get the code for user inputed escape code. This is not too hard to understand. What is wrong - is that these received POST data are decoded back to generate the tag href. But they should only be decoded back to generate the tag text. In contrast, in a case when forms do use «enctype="multipart/form-data"», then the browser does not encode the POST data. In this case, the tag text can be generated without any additional processing, and url-encoding must be used to generate the tag href. I may be wrong on the last paragraph, but this still doesn't change the problem. |
Ash88Jul 10, 2011 4:42 PM
|
| yes, data in the query string needs to be urlescaped -- that's the one of the main purposes of url escaping (along with characters outside the limited range allowed in urls). I don't think I contradicted anything you said.... I see what you mean about the hrefs in the tag links, since they should be represented as: http://myanimelist.net/animelist/username&tag=boobs%26guns with a php urlencode() on anything in the query string.The data in the forms is not actually handled as a form so you might be making some incorrect assumptions. If it were a traditional form the browser encodes form data once on submission pretty much always. The problem is that javascript is used to grab the fields instead of a form submit so the encoding has to be done manually (once automatically by jquery in the $.post() and not twice as it is currently). So there are two problems here: first the php that generates tag links needs to urlencode() the tags in the href, and second the javascript to add/edit tags should only encode the tags once automatically in the $.post(). The problems are still entirely on the list -- the former in the php and the latter in the js. |
I am a banana. |
saka said: I assumed this because you said about the POST data and the browser. I didn't really checked this in the code and thought that you know. Sorry :)The data in the forms is not actually handled as a form so you might be making some incorrect assumptions. If it were a traditional form the browser encodes form data once on submission pretty much always. The problem is that javascript is used to grab the fields instead of a form submit so the encoding has to be done manually (once automatically by jquery in the $.post() and not twice as it is currently). Anyway, I understand now from what you said that this is a php problem too, because the hrefs are actually generated with the php on a request. But I don't get what you meant by being encoded twice currently. As I see it, they are being decoded back, not encoded twice. Or was this what you meant by «twice»? Thanks for replying, btw. And please do something to organize a fix for this. If it's just one extra encoding that needs to be added than there shouldn't be too much trouble to fix this. It's a ridiculous bug for such a great site. |
|
| well, we have very little communication with the devs (essentially just Xinil, but crave has been fixing things recently also) and they just kind of pop in occasionally to fix what bugs they can. I misunderstood something with the "twice" there.... I thought you were implying they were doubly encoded or something, but that isn't the case of course -- they are just truncated because of url entities as you explained in your first post. My bad~ The problem with the js is indeed as you say... it uses $.get() to send the tags in the query string unencoded. In the tag_add() function in list.v16.js the following lines: function tag_add(aid,type) ...need to be changed to:{ var url; var tagString = document.getElementById("tagInfo"+aid).value; document.getElementById("tagRowEdit"+aid).style.display = 'none'; document.getElementById("tagLinks"+aid).style.display = 'block'; document.getElementById("tagLinks"+aid).innerHTML = 'Saving...'; document.getElementById("tagRow"+aid).innerHTML = tagString; if (type == 1) { url = "/includes/ajax.inc.php?t=22&aid="+aid+"&tags="+tagString; } else url = "/includes/ajax.inc.php?t=30&mid="+aid+"&tags="+tagString; //alert(url); $.get(url, function(data) { //alert(data); document.getElementById("tagLinks"+aid).innerHTML = data; document.getElementById("tagLinks"+aid).style.display = 'block'; document.getElementById("tagChangeRow"+aid).style.display = 'block'; } ); } if (type == 1) { url = "/includes/ajax.inc.php?t=22&aid="+aid+"&tags="+encodeURIComponent(tagString); } else url = "/includes/ajax.inc.php?t=30&mid="+aid+"&tags="+encodeURIComponent(tagString); And of course the php tag links would just need urlencode($tags); in a similar fashion inside the href. |
I am a banana. |
| Are you sure the "tagString" variable in the code above isn't the string containing all the tags for the row, separated with commas? The commas should not be encoded you know... |
|
| It doesn't matter that the commas are encoded, since they are decoded when received. The tags are passed to the php as a whole comma-separated string anyway. Tags cannot contain commas either for this reason. |
I am a banana. |
| But wouldn't the passed string be treated as one big tag if there would be "%2C" instead of commas? Not to mention leading and following spaces ("%20")... |
|
| if you submit the tags "foo, bar", it gets encoded as "foo%2C%20bar" and sent to ajax.php.... where php automatically decodes it back to "foo, bar" and then the string is split at the commas and the whitespace is trimmed off each. Well, it's likely the tags are stored as-is in a whole string and that is done on display... but either way~ the only needed changes are the ones mentioned in post 6 |
I am a banana. |
| You are right. I've tried now to submit "foo%2C%20bar", and it went just fine :) I'm looking forward to this fix. Please post here when there will be progress. |
Ash88Jul 11, 2011 2:33 AM
|
More topics from this board
» How to disable the Mai recommendation bot thingRhys2753 - 4 hours ago |
14 |
by Rhys2753
»»
34 minutes ago |
|
» HIstory doesn't show in order when mixed with anime and mangaTyphlosion101 - 10 hours ago |
0 |
by Typhlosion101
»»
10 hours ago |
|
» I had to create new account just to Logouthexploy - Jan 28, 2024 |
6 |
by baldursgay
»»
Yesterday, 9:33 PM |
|
» Blogs Max. Character Limit ?SukiiFu - Yesterday |
0 |
by SukiiFu
»»
Yesterday, 12:51 PM |
|
» Necromancer's Evolutionary TraitsKriegerBR - Yesterday |
0 |
by KriegerBR
»»
Yesterday, 10:02 AM |