Bug #575

Redundant Code in inc/class_parser.php

Added by Dylan Myers about 2 years ago. Updated over 1 year ago.

Status:Closed Start date:12/08/2009
Priority:Normal Due date:
Assignee:Ryan Gordon % Done:

100%
Category:Post Parser
Target version:1.4.12
Reproducibility:Always Database Type:MySQL
Reported In MyBB Version:1.4.10 Database Version:5.1.40
PHP Version:5.3.1 SQA assignments:
Browser:FireFox

Description

In inc/class_parser.php (1.4.10) on line 848 you have an if that should never be executed. It also as a bad preg_match().

Lines 838-851:

        if(!preg_match("#^[a-z0-9]+://#i", $url))
        {
            $url = "http://".$url;
        }
        $fullurl = $url;

        $url = str_replace('&', '&', $url);
        $name = str_replace('&', '&', $name);

        if(!preg_match("#[a-z0-9]+://#i", $fullurl))
        {
            $fullurl = "http://".$fullurl;
        }

As you can see, the preg_match() in the second if is missing the ^ character to match line start only, however it is a redundant if statement since the first one will have done this to $url before the $fullurl = url;.

Associated revisions

Revision 4623
Added by Ryan Gordon about 2 years ago

Fixes Redundant Code in inc/class_parser.php (fixes:575)

Revision 4676
Added by Ryan Gordon about 2 years ago

Fixes Redundant Code in inc/class_parser.php (fixes:575)

Revision 4678
Added by Ryan Gordon about 2 years ago

Fixes Redundant Code in inc/class_parser.php (fixes:575)

History

#1
Updated by Ryan Gordon about 2 years ago

  • Category set to Post Parser
  • Status changed from New to Assigned
  • Assignee set to Ryan Gordon
  • Target version set to 1.4.12

#2
Updated by Ryan Gordon about 2 years ago

SQA should also test that the following characters get escaped properly in urls:

'$' => '%24'
'^' => '%5E'
'`' => '%60'
'[' => '%5B'
]' => '%5D'
'{' => '%7B'
'}' => '%7D'
'"' => '%22'
'<' => '%3C'
'>' => '%3E'
' ' => '%20'

#3
Updated by Ryan Gordon about 2 years ago

  • Status changed from Assigned to Resolved
  • % Done changed from 0 to 100

Applied in changeset r4623.

#4
Updated by Chris W. B. about 2 years ago

The character $ doesn't seem be getting escaped in urls, but the rest work fine.

#5
Updated by Stefan T. about 2 years ago

  • Status changed from Resolved to Feedback

#6
Updated by Ryan Gordon about 2 years ago

  • Status changed from Feedback to Resolved

Applied in changeset r4676.

#7
Updated by Ryan Gordon about 2 years ago

Applied in changeset r4678.

#8
Updated by Michael Malin over 1 year ago

I have tested the following:

[url]http://www.dfds$.tld/[/url]
[url]http://www.dfds^.tld/[/url]
[url]http://www.dfds`.tld/[/url]
[url]http://www.dfds[.tld/[/url]
[url]http://www.dfds].tld/[/url]
[url]http://www.dfds{.tld/[/url]
[url]http://www.dfds}.tld/[/url]
[url]http://www.dfds".tld/[/url]
[url]http://www.dfds<.tld/[/url]
[url]http://www.dfds>.tld/[/url]
[url]http://www.dfds .tld/[/url]
[url]http://www.dfds–.tld/[/url]
[url]http://www.dfds™.tld/[/url]

The last two URL's I added to my own discretion.
And [url]http://www.dfds".tld/[/url] is not linked, despite the URL tags.
The following are linked not correctly:
[url]http://www.dfds<.tld/[/url] > Direct
[url]http://www.dfds>.tld/[/url] > Direct
[url]http://www.dfds–.tld/[/url] > Links to http://www.xn--dfds-386a.tld/ ???
[url]http://www.dfds™.tld/[/url] > Links to http://www.dfdstm.tld/

#9
Updated by Ryan Gordon over 1 year ago

Those are not valid domain names. You cannot register a domain name like that so it naturally shouldn't parse.

#10
Updated by Michael Malin over 1 year ago

So this bug is resolved and should be closed?

#11
Updated by Ryan Gordon over 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF