ShortCodeParser: A parameter value of 0 is ignored by the parser and fails.

[ghost]

This type of shortcode:
[SendaContent id='0']
produces the following error:

[Notice] Undefined offset: 2
GET /

Line 222 in C:\wamp\www\framework\parsers\ShortcodeParser.php
Source

213 
214                 // Pull the attributes out into a key/value hash
215                 $attrs = array();
216                 
217                 if (!empty($match['attrs'][0])) {
218                     preg_match_all(self::attrrx(), $match['attrs'][0], $attrmatches, PREG_SET_ORDER);
219 
220 
221                     foreach ($attrmatches as $attr) {
222                         list($whole, $name, $value) = array_values(array_filter($attr));
223                         $attrs[$name] = $value;
224                     }
225 
226                 }
227                 
228                 // And store the indexes, tag details, etc

Trace

    ShortcodeParser->extractTags([SendaContent id='0'])
    ShortcodeParser.php:367
    ShortcodeParser->replaceElementTagsWithMarkers([SendaContent id='0'])
    ShortcodeParser.php:522
    ShortcodeParser->parse([SendaContent id='0'])
    SendaConfig.php:1455
    SendaConfig::ContentParser([SendaContent id='0'],Page)
    SendaPageDecorator.php:247
    SendaPageDecorator->UpdateContentAndSearch(,,,,,,)
    Object.php:989
    Object->extend(UpdateContentAndSearch,)
    Page.php:12
    Page->UpdateContentAndSearch()
    SendaConfig.php:1892
    {closure}()
    call_user_func(Closure)
    Fluent.php:465
    Fluent::with_locale(en_US,Closure)
    SendaConfig.php:1897
    SendaConfig::CheckOperations()
    SendaConfig.php:1512
    SendaConfig::Init(Page_Controller)
    Page.php:39
    Page_Controller->init()
    Controller.php:139
    Controller->handleRequest(SS_HTTPRequest,DataModel)
    ContentController.php:220
    ContentController->handleRequest(SS_HTTPRequest,DataModel)
    ModelAsController.php:68
    ModelAsController->handleRequest(SS_HTTPRequest,DataModel)
    FluentRootURLController.php:95
    FluentRootURLController->handleRequest(SS_HTTPRequest,DataModel)
    Director.php:325
    Director::handleRequest(SS_HTTPRequest,Session,DataModel)
    Director.php:143
    Director::direct(/,DataModel)
    main.php:189

This occurs for this code:
function extractTags($content) (ShortcodeParser.php)

list($whole, $name, $value) = array_values(array_filter($attr));

since the variable $attr contains the following array:

$attr [3]
  0 = "id='0'"
  1 = "id"
  2 = "0"

And the following code modifies the array

array_values(array_filter($attr));

Now the array contains only two elements:

0 = "id='0'"
1 = "id"

which makes the error occurs when assigned to

list($whole, $name, $value)

Please, fix this problem as soon as possible.

Thanks,
Regards,
Jose A.

[phptek]

I'd be interested to see the custom-logic you had written that requires something with an id of '0' in it. I'm not being dismissive, but need to ascertain if this is going potentially be a problem for other devs.

FYI I can replicate simply by linking to another page in the CMS, manually updating the ID to be '0' and viewing that page's front-end, just not sure at this point, what other scenarios would result in the same issue.

FYI the observed behaviour is a direct result of running array_filter()over the $attrarray, it filters out any array-values that equate to false in PHP e.g. zero, empty strings, null etc.

[phptek]

Am unsure at the moment on what to do here. On the one hand there is no evidence (yet) of how other's may be affected by this. On the other hand the logic itself isn't the best - allowing assignment via the list() construct after knowingly filtering out values that equate to FALSE.

[hafriedlander]

$attrrx has three possible options (value surrounded by ', by " or bare), and the array_filter was supposed to cut that down to just the one actually provided, but I had forgotten about false-ish values. Just pass in a callback that doesn't convert type (so === null or === "", I'm not sure what one preg_match provides for a non-matching capture group).

[phptek]

@hafriedlander yup, experimenting with the callback as I write.

[phptek]

@hafriedlander when you get time (!!) please review the two tests in c154bbf, particularly testOneFalseishArgumentWhereIsNotZero() which fails, even prior to applying the simple fix that comprises #2382. Basically, I'm unsure at what point during parsing a string comprising "" or '' is OK or not OK.

Many thanks.

[ghost]

@hafriedlander I 've tried setting the value to '0' or "0", but does not work . Produces the same problem.
@phptek Whatever you can do using values ​​0, the system should allow parameters set to a value of 0 without fail.

In my system, specifically, the values ​​assigned by an end user to custom descriptions take values from 1..., and the value 0 is the default description.

But this kind of problem can be presented to any user who has to use a parameter with value 0.

This is clearly a bug.

I do not think that other users have many problems with this issue because of SilverStripe shortcodes system is too limited to do more complex things and I do not think that many people are using it. Ideally, future Silverstripe versions would do a better system shortcodes.
At the time that you want to do more sophisticated things , you see all their problems and limitations. :(

Thank you for your efforts to solve this problem.

Regards,
Jose A.

[phptek]

@Josua2012 it's OK, I think it's a bug too :-)

The simple fix in fe7b810 sorts it out, I'm just waiting for feedback from the folks who understand this part of the system better than I, whether the accompanying tests are good to go.

[chillu]

@hafriedlander Does this look good to you?

[hafriedlander]

Yes, it's just missing an edge case. See my comments in the Pull Request - #2417

I though @phptek might be able to update the PR quickly, but I guess he ran out of time? We can probably merge in as-is if no-one has a chance to fix the edge case any time soon.

[phptek]

Yeah sorry guys, am right in the middle of a mini home-reno to get the house on the market. I will follow up with any edge cases, if still required next week. FWIW the specific issue raised here is fixed by the related PR.

[td204]

This issue is still around in 3.1.15: the fix in #2417 works for me.

[sheadawson]

This is an issue for me still also

[hafriedlander]

#2417 still needs a fix before it can be merged - as it is, it fixes a subset of the larger issue (="0") but not the whole issue (="" would still break). If someone wants to find some time to fix the PR, the principal behind it seems fine.

[tractorcow]

I'll see about fixing this up for the 2.2.2 release (and 3.3.0)

[dhensby]

Bump

[tractorcow]

I'm not gonna have time I think. :) Too late to get this into 3.3.0.

[dunatron]

This issue is still around

[Firesphere]

I'll have a look for SS4, seems to be the too broad empty check.

[tractorcow]

It was fixed with 8ae794e#diff-419fde14ad0518fd3b25838354c25674L265 in 4.x.

[dhensby]

Fixed in 3.2+ with #5761

Note that @hafriedlander's example edgecase of [shorcode id=""] actually returns array('id' => '""') so I didn't address it as that's maybe kind of intended maybe probably?