Welcome to the Question2Answer Q&A. There's also a demo if you just want to try it out.
+3 votes
1.7k views
in Q2A Core by
Hello All ,

Most of us are using IP Blocking feature of q2a .

All the ip addresses you block that will be stored in the block_ips_write field in qa_opttions table

So as per the column has maximum length of the table value column we can store upto 8000 chars .

Once it is full if you block ip addresses it will not work . The ip address will not be stored in the field . And it will look like the blocking mechanism unfunctional .

We can not say it as a bug , rather it is a limitation .

 

What you say . Let me know your thoughts .
Q2A version: 1.6.3
by
In files if the chars limit can be increased then that should be done. Although this feature should not be used but there is no other way to stop spam at the moment. The only way out is to increase the chars limit, this is what I think.

1 Answer

+2 votes
by

A bug is a feature of an application that doesn't match the original requirements. Requirements in Q2A are implicit and I think the feature is not a bug as the code works as how it seems it is intended to work: IPs are stored in that table and are ruled by the character limit of the field.

Having said that, I don't like how it is implemented now. There are very simple improvents that could be made to the code including not imploding with ' , ' (with 2 unnecessary spaces), removing the ASCII representation of the IPs and use integers (4 bytes per IP) and store them as BLOBs and, probably the best approach in my opinion, deprecate that field and start using a separate table that would only store those IP addresses.

Here are some approaches I'm thinking right now:

Efficient alternatives that are complex or might not provide full feature support: The latter solution does have a downside which is it will not support ranges or asterisks. Ranges can be supported by adding a second IP address column to represent a FROM and a TO. In order to support asterisks and using only integers I think the only way would be to split the integers into 4 bytes and process them separately in the query WHERE clause (kind of awful code)

Least efficent alternatives that would provide full feature support: Just deprecate the field and add al the IP addresses to a separate table as a character field the same way as it is now. The whole table would have to be read always to check for an IP address in there

Mix of both approaches: Use the least efficient one but also add a FROM and TO IP addresses. The idea would be to treat them all as ranges so you search for the IP range using an integer index and when you find a match then you use the character field to discard non-matches generated by asterisks like 123.*.123.123. I believe this should be pretty efficient as the asterisk will usually be on the right side of the string which would allow the index to filter out more data

I would go for the first one (using FROM and TO IP addresses as 2 columns) and remove the asterisk support from the core but leave the ranges so that you could block 123.123.123.100 - 123.123.123.255 but you won't be able to block 123.*.123.123

by
Yeah it doesn't make any sense to have wildcards in the middle. For example if 123.123.*.* blocked one country, 123.123.*.123 would block 1 in every 256 people, randomly scattered across the entire country.

While using one field to store IP addresses has its limitations, it is useful since they are fetched in one query with all other options. Also 8000 characters will allow you at least 500 IPs. If we have to load 500 (and increasing) rows from another table on every page load and check against them, seems like it will get very slow.

Problem is with most spam, they use some kind of proxy and then move onto another so an IP you blocked 6 months ago is no longer doing anything. I guess it might be worth logging the last time an IP address visited the site, then you can remove the old ones.

The solution for amiyasahu is probably to combine those 500 IPs into some wildcard addresses.
by
"If we have to load 500 (and increasing) rows" => Oh, I mean not loading but just checking if the current IP address hitting the site is blocked or not. Having an indexed ranged pair of IP addresses will allow to return the matching of the IP in a given range in no time. I haven't taken a look at the core but I guess that is pretty much what it does... currently the asterisk will force the processing of each IP string separately so that it becomes something comparable so that's why currently all IPs need to be fetched. Otherwise, I guess true/false should be more than enough

Regarding expiring IP address ranges that could be parametrized. So the user could say "I want IP address ranges to expire after X days" and, in order to avoid a cron there, it could be added as one of the DB cleanup operations. This would only require adding a date field to that table

Whether blocking IP addresses will stop spam or not I think that's a related but separate (and quite hot) matter
by
Yah I agree that it is not a bug of Q2A . Thanks both of you for your suggestions :)
by
Oh so you mean just running some SQL like
SELECT * FROM qa_blockedips WHERE [ip] >= fromip AND [ip] <= toip

Yeah that makes sense and I think would be fairly efficient. For 99% of users that will return nothing.
by
That's right. Actually :P
SELECT 1 FROM qa_blockedips WHERE [ip] BETWEEN fromip AND toip LIMIT 1

The table should have a compound key in fromip and toip. The PHP code will be trivial and would actually be considerably more simple than it currently is as there wouldn't be any parsing
...