[F] Admincp IPaddress query problem

[User 2877]

$query = $db->simple_select("posts", "DISTINCT ipaddress,pid", "uid='{$mybb->input['uid']}'");


That's the query from /admin/modules/user/users.php on line 1398 (at least of the new file I have).

The problem is that this repeats IPs and actually make a pretty big query result if a member has thousands of posts and actually choked my large forum. The pid from DISTINCT should be removed as it's not needed for this query.

$query = $db->simple_select("posts", "DISTINCT ipaddress", "uid='{$mybb->input['uid']}'");

My tests show this as working perfectly well but of course dev team should confirm.

Oh...a couple lines down I see where the pid is used to create the unique identifier for the popup. You can substitute it with the IP address though.

$popup = new PopupMenu("post_{$ip['ipaddress']}", $lang->options);

Again..seems to be working perfectly fine under my tests.

Thanks and I hope this gets added to next release.

Since I have 1.4x on hackforums.net now I am finding lots of things that could use improvement. It's a big site and requires a lot of admincp moderation. So far mybb 1.4x is doing well overall but certain parts are slow or clunky. I will help with fixes and suggestions as best I can.

Thanks.

[Ryan Gordon]

Having an html id with decimals?

[User 2877]

Yeah..I know but it seemingly works.

Worse case what about a simple count loop for id#?

Or how about doing a str_replace and replacing them with _ (underscore)?

The problem with pid is that if a user has thousands of posts...well..if you have admincp access here just search my own ipaddress. I probably have just one but you will get a listing of the same ip one for each of my posts. If a board has 500k posts and a user has 5000 posts...this is going to be a real issue for speed on that query. Already it's choking with 1/2 those stats.

Let me know what you come up with.

ADD: Trying to find allowed legal characters for id attribute. So far this is all I found.

http://www.w3.org/TR/xhtml1/

Note that the collection of legal values in XML 1.0 Section 2.3, production 5 is much larger than that permitted to be used in the ID and NAME types defined in HTML 4. When defining fragment identifiers to be backward-compatible, only strings matching the pattern [A-Za-z][A-Za-z0-9:_.-]* should be used. See Section 6.2 of [HTML4] for more information.

Seems to signify the period is an acceptable character.

And the page does validate just fine. I noticed in source that it says post_xx.xx.xx.xxx so of course the post should be changed to ip I guess as well to be consistent.

Thanks.

[Ryan Gordon]

Yeh, I don't think we even use the id's for anything. Could probably just use a simple incrementer

[User 2877]

Without the IDs the form for dhtml won't work. So they have to be unique. An incrementer built into mybb for this would be useful for admincp. I am sure you will figure it all out. Just wanted to point out these issues of the results and possible fixes.

[Dennis Tsang]

Yup pid should be removed from the distinct, or else everything is distinct because pid is the primary key.

Ok I just had a look at that code, and it's quite convoluted -- the DISTINCT is useless, and the $doneip which is supposed to mimic DISTINCT has a typo which renders it also useless . My proposed solution is to replace:

        $query = $db->simple_select("posts", "DISTINCT ipaddress, pid", "uid='{$mybb->input['uid']}'");
        while($ip = $db->fetch_array($query))
        {
                if(!$done_ip[$ip['ipaddress']])
                {
                        $popup = new PopupMenu("post_{$ip['pid']}", $lang->options);
                        $popup->add_item($lang->show_users_regged_with_ip, "index.php?module=user/users&results=1&action=search&conditions=".urlencode(serialize(array("regip" => $ip['ipaddress']))));
                        $popup->add_item($lang->show_users_posted_with_ip, "index.php?module=user/users&results=1&action=search&conditions=".urlencode(serialize(array("postip" => $ip['ipaddress']))));
                        $popup->add_item($lang->ban_ip, "index.php?module=config/banning&filter={$ip['ipaddress']}");
                        $controls = $popup->fetch();

                        $table->construct_cell($ip['ipaddress']);
                        $table->construct_cell($controls, array('class' => "align_center"));
                        $table->construct_row();
                        $done_ip[$ip['ipaddres']] = 1;
                }
        }

with this:

        $query = $db->simple_select("posts", "DISTINCT ipaddress", "uid='{$mybb->input['uid']}'");
        while($ip = $db->fetch_array($query))
        {                $popup = new PopupMenu("post_".preg_replace('#[^a-z0-9]#i', '_', $ip['ipaddress']), $lang->options);
                $popup->add_item($lang->show_users_regged_with_ip, "index.php?module=user/users&results=1&action=search&conditions=".urlencode(serialize(array("regip" => $ip['ipaddress']))));
                $popup->add_item($lang->show_users_posted_with_ip, "index.php?module=user/users&results=1&action=search&conditions=".urlencode(serialize(array("postip" => $ip['ipaddress']))));
                $popup->add_item($lang->ban_ip, "index.php?module=config/banning&filter={$ip['ipaddress']}");
                $controls = $popup->fetch();

                $table->construct_cell($ip['ipaddress']);
                $table->construct_cell($controls, array('class' => "align_center"));
                $table->construct_row();
        }   

Can you quickly test if this works (particularly the regexp part that replaces non-alphanumeric characters with an underscore).

[User 2877]

Try to do some testing with this soon as I get a chance. It's going to be a busy weekend for me. If you have a deadline for bug fixes in 1.4.3 just give me a PM so I can start testing and working on a lot of bugs. I want to help as much as I can as it appears you guys are a bit overworked right now. Trying my best to help out while I have some time.

[Ryan Gordon]

We do definately appreciate the efforts labrocca

[Dennis Tsang]

Actually as you said, the PopupMenu name isn't really significant to anything right? (plugins can't hook into it) so maybe I'll just replace the preg_replace with a counter++.

[maritrench]

I'm getting what I think is a related error. Tried the code change above and it didn't break anything, user IP lookup seems to be working fine, but it had no impact on my issue... so assuming it's a similar-but-not-same issue: When I try to find users who have posted with a given IP address, I get the entire membership list. I'd try to sift through the code myself, but I'm not even close to comfortable with doing that yet.

Thoughts? Thanks.