ElkArte Community

On this topic: http://www.elkarte.net/index.php?topic=443.30

It doesn't not show me the "new" tag beside any topics in the MessageIndex, but the BoardIndex it is in(Site Feedback) shows as bold(as in new topic/reply).

Other topic in other boards have the "new" label if there is a new topic/reply.

Yep, noticed, but I didn't have time to debug it (I was breaking other things :P).

I think it is the first time you enter a board (i.e. if you have two boards with unread messages, the first you enter doesn't give the "new" message, while the others do), but I'm not 100% sure.
If it is so, it may be related somehow to the session.

I haven't been as involved with the community since this issue. It seems this issue is still there. I don't visit everyday so I can't tell which posts are new per topic or not.

Yep, still there.
I think (not sure though) it has been solved in the repo, but the site is kind of outdated... O:-)

Seems to be fixed now. Thanks.

It's fixed on the site, but is still broken in the code.
The issue is in the  Request class, where $_REQUEST['start'] is always considered an integer, while in few places it is not.

If you want I'll make a PR for that ... but basically I removed that limitation vs change all the code to enforce that limitation.  Otherwise we can wait for Norv to have some free time since that was her code change and she should know best which direction.

I don't know exactly where put my hands to fix it "properly", mostly because there are at least two "exceptions" (to the (int) rule) and both are slightly different as far as I've seen, so they would require an ad hoc implementation of the cleanup.

If the do-not-cast-to-int is enough for now, I can push it to the branch I have open or if you want feel free to push it in your PR! :P

/me likes the second option because is less work for him. :P

Feel free to make the PR :) ... for now I'd go with the do not cast solution since as you noted, the other fixes are quite a bit more involved and lets face it, its been un-cast for a heck of a long time  O:-)  (Does not mean its proper, just that it does not seem harmful)

https://github.com/emanuele45/Dialogo/commit/329518e18fa6d25d01baa799ccf0b49b73b10e5b

Commented out only for Display, because apparently all the others are int...apparently.

Does it need to be set to zero if its not set ?  What I did on the site was:

$_REQUEST['start'] = isset($_REQUEST['start']) ? $_REQUEST['start'] : 0;

There is a check (luckily, I didn't see it at first :P) at line 262 (https://github.com/emanuele45/Dialogo/blob/master/sources/Request.php#L262) that seems to take care of that. O:-)

Maybe you could have a little regex for that. I don't remember the exceptions but something like this:

$_REQUEST['start'] = isset($_REQUEST['start']) && preg_match('~^(:?(:?from|msg)?\d+|new)$~', $_REQUEST['start']) ?  $_REQUEST['start'] : 0;

A little test code too, if you can think of more cases:

<?php

$regex = '~^(:?(:?from|msg)?\d+|new)$~';

$tests = array(
 '123' => 1,
 'new' => 1,
 'msg123' => 1,
 'from123' => 1,
 '123msg' => 0,
 'from12x3' => 0,
 'new123' => 0,
 'news' => 0,
 'msg1.23' => 0,
);


echo '
<style>
table
{
 border-collapse:collapse;
}
td, th
{
 border: 1px solid #000;
 padding: 0.5em;
}
</style>
<table>
 <tr>
 <th>Test</th>
 <th>Expected</th>
 <th>Result</th>
 </tr>';

foreach ($tests as $test => $expected)
{
 $result = preg_match($regex, $test);
 echo '
 <tr>
 <td>', $test, '</td>
 <td>', $expected, '</td>
 <td>', $result, '</td>
 </tr>';
}

echo '
</table>';

Reminds me of the time we were coding compareVersions() and matchPackageVersion() functions right before 2.0 release with Norv and came up with 100+ cases to test. Good old days...


That turned out to be a pretty weird check but interestingly it actually prevented a PHP bug to be exploited.

Thanks [SiNaN]!
Will try for sure.
I don't know exactly all the exceptions me either, but if we find some we can add them to the list. :P

Norv wrote it, you can't expect anything less than weird checks. :P

They are probably all there is, from what I can see in the part related to the handling of start variable in Display controller. But, yeah, you'll know for sure once people start complaining about it, lol.


Actually it was me who wrote that, lol. I was tempted to pretend to know nothing but felt sorry for Norv...

ohhh... lol
Then you are as weird as Norv and I didn't know. :P

May I take that as a compliment? :P And that explains why we got along so well with Norv. lol

Committed locally.