Kroc/NoNonsenseForum

THE INSANITY BUG: Post IDs beginning with 'd' cause two letters to be eaten in the append URL

Closed this issue · 5 comments

Kroc commented

This bug is enough to drive a person mad. There is no sane explanation, I can't narrow it down.

If a post ID beings with a d, the append link becomes broken because ?append=dxyz turns into ?appendxyz. Example: Check the append link on this post http://forum.camendesign.com/for_programmers_cleaning_up_user_display_names+1#dngbsvpzz7i

I believe this also happens with the delete link where the post ID begins with an e.

What I know:

  • It's not the url function, output from that is clean
  • Whilst it could be PHP XML DOM, at the point DOMTemplate sets nodeValue, the value is correct, and reading the node back is also correct

Three (read them, three) whole Internet Points to anybody who can solve this.

Delapouite commented

Hi Kroc.

it seems that your bug has something to do with this loop :
https://github.com/Kroc/DOMTemplate/blob/master/domtemplate.php#L403

//convert XML style attributes (`<a attr="attr">`) to HTML style attributes (`<a attr>`),
//this needs to be repeated until none are left as we must anchor each to the opening bracket of
//the element, otherwise content text might be hit too
while (preg_match ('/(<(?!!)[^>]+)([a-z-]+)=([\'"]?)\2\3/i', $source, $m, PREG_OFFSET_CAPTURE))
$source = substr_replace ($source, $m[1][0].$m[2][0], $m[0][1], strlen ($m[0][0]))
;

Try to comment this out and it should work. I didn't have time to dig in why this regex is guilty yet.

Delapouite commented

On a side note :
<meta http-equiv="X-UA-Compatible" content="IE=edge" >
is also turned into
<meta http-equiv="X-UA-Compatible" content="IEdge" >

So everytime there's a x=x pattern it gets stripped.

Zegnat commented

I didn’t have time to dig in why this regex is guilty yet.

Very simple actually, the regular expression doesn’t check where the = sign occurred within an element. Any = after a <, without any > between them, is matched. This includes = signs within attribute values.

One thing that could be done is to force matching white space before the attribute name like so:

'/(<(?!!)[^>]+)\s([a-z-]+)=([\'"]?)\2\3/i'

This way it is forced to match the complete attribute name and no longer just the end.

Kroc commented

You have a problem. You use regex to solve it. You now have two problems.

Massive thanks to both of you for for looking into this, esp. Delapouite for narrowing it down. I'm stretched so thin right now, this bug was driving me mad. Now I know where to look I can work this out. What name would you like to be credited with in the README, Delapouite?

Delapouite commented

Glad to help this project! My real name in the README will be fine. Thanks.