modolabs / Kurogo-Mobile-Web

Kurogo is a PHP framework for delivering high quality, data driven customizable content to a wide range of mobile devices. Its strengths lie in the customizable system that allows you to adapt content from a variety of sources and easily present that to mobile devices from feature phones, to early generation smart phones, to modern devices and tablets
http://kurogo.org
GNU Lesser General Public License v2.1
198 stars 99 forks source link

ICSDataParser does not handle ALTREP properly. #34

Closed eebs closed 12 years ago

eebs commented 12 years ago

The ALTREP param of DESCRIPTION is not handled properly.

DESCRIPTION;ALTREP="http://www.example.com/somepath/":This is an example description.

The regular expression on line 42 of lib/Calendar/ICSDataParser.php does not account for a colon in a URI as part of a parameter:

preg_match('/(.*?)(?!<\\\):(.*)/', $line, $parts)

The ALTREP spec states that the URI must be in quotes, so to fix this error the regex should not match a colon if it inside double quotes. Unfortunately my skill with regex is not up to par, so I leave the actual solution up to you.

Currently the array passed back by contentline() looks like this if the DESCRIPTION line contains an ALTREP:

Array
(
    [name] => DESCRIPTION
    [value] => //www.example.com/somepath/":This is an example description.
    [params] => Array
        (
            [ALTREP] => http
        )

)

I would also be interested in knowing why the regex tries to match anything before the colon unless it is followed by <. What is the case that this is preventing?

eebs commented 12 years ago

Well, I've found a solution to the problem. If I assume the positive lookahead does nothing (I can't find anything in any iCal files that have that pattern), it can be removed, and the preg_match can be changed to:

preg_match('/([^":]*(?:"[^"]*"[^":]*)*):(.*)/', $line, $parts)

This pattern works on all feeds we use, and as far as I can tell splits the name, value, and parameters correctly.

The array passed back by contentline() now correctly looks like:

Array
(
    [name] => DESCRIPTION
    [value] => This is an example description.
    [params] => Array
        (
            [ALTREP] => http//www.example.com/somepath/
        )

)

I seem to have a pull request waiting and I can't figure out how to submit another pull request without the first one going away (seems it's not possible), so I'll wait to submit a pull request.

akinspe commented 12 years ago

I have merged your other pull request. Please feel free to submit one for this issue