Advertisement:

Author Topic: [MOD][PENDING] Spam Blocker - Anti-spam modification to restrict or limit access  (Read 88798 times)

Offline NanoSector

  • Customizer
  • SMF Super Hero
  • *
  • Posts: 10,471
  • Gender: Male
  • VC321xb47@aperture:~#
    • Yoshi2889 on GitHub
Why would we create a function to insert data when you can do the same in a query?

$smcFunc['db_insert'] performs additional checks and routines on the data inserted which $smcFunc['db_query'] doesn't do.

If $smcFunc['db_insert'] didn't add anything to manually inserting data with _query then we wouldn't mind, but since it does...

  Yoshi,
  I filter all the data prior to insertion, be it text or integer. I always make sure the data is safe prior to the insertion therefore I do not understand why I am being forced to change it.
Even though you sanitise the data you still need to use db_insert, end of discussion. Change it if you want your mod approved.
My Mods / Mod Builder - A tool to easily create mods / Blog
"I've heard from a reliable source that the Answer is 42. But, still no word on what the question is."

Offline Chen Zhen

  • Sophist Member
  • *****
  • Posts: 1,027
  • Gender: Male
  • If you're going through hell, keep going!
    • Underdog-01 on GitHub
    • WebDev.ca
Even though you sanitise the data you still need to use db_insert, end of discussion. Change it if you want your mod approved.

  Sadly, your mannerism does not surprise me.



re. Spam Blocker_v1.0-RC1.5x.zip

Changes:
! $smcFunc['db_insert'] implemented

Offline Colin

  • Lead Developer
  • SMF Hero
  • *
  • Posts: 7,760
  • Gender: Male
  • SMF Developer
    • colinschoen on GitHub
Hi Underdog,

I can see your frustration, but there is a reason for standardization. Let me try to clear it up, if I may. If for example, SMF changes the method of sanitation of data down the road, we would want it to apply to all modifications. Your mod does mimic the function now, but the functions are evolving and we want your mod to be used in the community for as long as possible. Thank you so much for your understanding.
"If everybody is thinking alike, then somebody is not thinking." - Gen. George S. Patton Jr.

Colin

Offline Chen Zhen

  • Sophist Member
  • *****
  • Posts: 1,027
  • Gender: Male
  • If you're going through hell, keep going!
    • Underdog-01 on GitHub
    • WebDev.ca

Colin,

  Now I understand the reasoning perfectly & I thank you for that explanation.

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 68,032
    • Arantor on GitHub
Actually, it's nothing whatsoever to do with sanitisation, since the same sanitisation protection is really employed at the query level anyway through the parameterisation system.

db_insert does a bunch of things that db_query won't do for inserting on non-MySQL platforms. It does all kinds of handling around the primary and foreign keys on both PostgreSQL and SQLite, and there's a bunch of stuff being mimicked for getting the last inserted id.
To assume is to hope that those who came before had the presence of mind and capacity to implement the dreams of those who would come after.

You either die a hero or live long enough to see yourself become the villain. It seems you have chosen which, and now I must do the same.

Offline phill104

  • Semi-Newbie
  • *
  • Posts: 13
Underdog,

Many thanks for the mod, it seems to be working well for us. One thing I feel that could improve it is the facility to check members that are already registered. I am aware you can manually enter an IP and an Email address in the box but that is time consuming to copy the details from the members page into the box on the mod. Would it be possible to add a button on the members page that jumps straight to the manual entry page of your mod with the users details already filled out?

Offline Chen Zhen

  • Sophist Member
  • *****
  • Posts: 1,027
  • Gender: Male
  • If you're going through hell, keep going!
    • Underdog-01 on GitHub
    • WebDev.ca
Arantor,

  Yes I am aware it was asked so that it will be compatible with various database types although the mod does state that the recommended requirement is MYSQL.  It is my experience that the $smcFunc['db_insert'] function does not seem to allow me to change just 1 or a few columns of data whereas I seem to be forced to reading a whole row of data, make the necessary changes and then use the db_insert function to replace that row. Am I wrong here or does the current smcFunc functions not include an equivalent for UPDATE (single+ columns .. less than all of them) that works for various db syntax?

phill104,

  Yes I will add that option and it is a very good idea.  Thank you for showing interest in the mod.

Regards.



Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 68,032
    • Arantor on GitHub
No, $smcFunc['db_query'] should be used for UPDATE for which a simple function can't be made. If you'd like to see what a db_update function would look like, I invite you to the joy farm that is Ruby On Rails.

db_insert is for inserting new rows or completely replacing rows, seeing how it can do either INSERT or REPLACE queries (or, in some backends' case, DELETE followed by INSERT)

If you're adding new rows or completely replacing existing rows based solely on their primary key, db_insert should be used, especially as you can add rows en masse.

If you're not adding complete rows, db_query is the way to go. Remember that an insert can work without supplying every column if appropriate defaults are used.
To assume is to hope that those who came before had the presence of mind and capacity to implement the dreams of those who would come after.

You either die a hero or live long enough to see yourself become the villain. It seems you have chosen which, and now I must do the same.

Offline Chen Zhen

  • Sophist Member
  • *****
  • Posts: 1,027
  • Gender: Male
  • If you're going through hell, keep going!
    • Underdog-01 on GitHub
    • WebDev.ca
Quote from: Arantor
If you're adding new rows or completely replacing existing rows based solely on their primary key, db_insert should be used, especially as you can add rows en masse.

  Yes, a primary key or composite keys. I am aware of this but what I am asking is can I use UPDATE in a query and have it compatible with the various db types or should I be changing any instances of UPDATE to smcFunc db_insert to make it compatible? I am very familiar with MYSQL but have no experience with other database types at this time to know if the syntax is the same across the board.

 

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 68,032
    • Arantor on GitHub
Yes, you should be able to use UPDATE for other backends via db_query as normal. The syntax should be comparable enough, just provided you don't do multi-table updates (MySQL can sometimes do that, the others can't)
To assume is to hope that those who came before had the presence of mind and capacity to implement the dreams of those who would come after.

You either die a hero or live long enough to see yourself become the villain. It seems you have chosen which, and now I must do the same.

Offline Chen Zhen

  • Sophist Member
  • *****
  • Posts: 1,027
  • Gender: Male
  • If you're going through hell, keep going!
    • Underdog-01 on GitHub
    • WebDev.ca

  On that note, I have no idea why they are neglecting to accept this modification as I implemented the mandatory changes that were asked. Oh well, let it collect dust in this board.
 

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 68,032
    • Arantor on GitHub
Because you generally have to send a PM saying you've fixed everything then it goes back in the queue. Then someone has to review it etc. There was a time you'd usually get same day service.
To assume is to hope that those who came before had the presence of mind and capacity to implement the dreams of those who would come after.

You either die a hero or live long enough to see yourself become the villain. It seems you have chosen which, and now I must do the same.

Offline Chen Zhen

  • Sophist Member
  • *****
  • Posts: 1,027
  • Gender: Male
  • If you're going through hell, keep going!
    • Underdog-01 on GitHub
    • WebDev.ca

  I did that back in June and have not heard anything since. I did update it a few times since but I'm told I can do that whilst its in the queue and nobody looked at any of those files. I sent another PM just a few minutes ago and will await a response. Someone downloaded from the dl site after our conversation here so perhaps it is now being looked at.
 

Offline Suki

  • Customizer
  • SMF Super Hero
  • *
  • Posts: 15,083
  • Kaizoku Jotei
    • MissAllSunday on GitHub
    • SMF mods
Just a few suggestions:

When searching for text as reference, try not to search for the entire line with tabs at the beginning, this search for example:

<![CDATA[               'notes' => $row['notes'],]]>

Can be done the same way without the tabs at the beginning:

<![CDATA['notes' => $row['notes'],]]>

You could use sprintf instead of str_replace: PHP String Replacement Speed Comparison

Not really mandatory but would be nice if your queries would be formatted to increase readability. Also, would be good to use the casting array and to check for every var existence before passing it to the query. Yes, most of the queries here have values that doesn't have to be empty but still, better safe than sorry.

Prob not a good idea to expand compatibility to, 2.99.99 its is better to keep it down to 2.0.99, 2.1 is still to young and could still have a bunch of changes, specially in templates, plus, we don't even know how SMF 2.95 would look like!

Suggestion, keep the same folder format SMF uses, a sources folder and a Themes folder, that way you only need to move both folders on install and helps things to be more organized specially for big mods with lots of files such as this one.

Watch your tabs and spaces on empty lines and at the end of text. Things like

echo '-->-->-->-->-->                               
--->-->--><a id="spamLink_' . $item['id']

Would be printed as it is by PHP.
Look at them. They're just asking for it. Maybe the human race deserves to be wiped out.

Offline Chen Zhen

  • Sophist Member
  • *****
  • Posts: 1,027
  • Gender: Male
  • If you're going through hell, keep going!
    • Underdog-01 on GitHub
    • WebDev.ca
Suki,

  I appreciate the suggestions but some of what you say is a matter of personal opinion. 

  • When searching for a line of code I like to include the preceding tabs (the whole line if you will) and I will always continue to do that.
  • str_replace is still one of the fastest methods
  • The db queries are tabbed out nicely and I do not understand the issue you speak of. Atm the queries you seem to be speaking about are inserts from SpamBlocker.php? It first inserts some data into an auto increment table (ban_groups) and then reads the id of that insertion prior to writing to the ban_items table. Since both normally work in unison, I deemed that unnecessary for the ban_items table although it could be implemented and then use conditional logic to opt insert or replace. As for the spamblocker_blacklist table it attempts a deletion first which was actually a shorter coded preliminary query than what you speak of & would also work for the ban_items table if I add that.
  • 2.09.99 .. I agree with that and already thought of it prior but neglected to change it.  I think something was changed regarding the hooks in 2.1 and you are correct this won't work with it as I believe I tested that some time ago. Still, this version description claims SMF 2.0.X compatibility and what is described to this mod's users is correct.
  • Themes and Sources folders in the installer... ya something I thought of prior also and did not do yet although I will change that.
  • I don't know what you are referring to regarding tabs and spaces. I use Komodo for files as my editor which is a proper code editor (has settings for each file type). It is set to 8 for the tabbing (4 or 8 is the norm & up to preference).. I can not see any issues regarding tabbing, to what are you referring?

Regards.


Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 68,032
    • Arantor on GitHub
Quote
When searching for a line of code I like to include the preceding tabs (the whole line if you will) and I will always continue to do that.

This is largely a matter of style. There are times it can make a difference. I personally wouldn't care about it (and I used to be the most of a hard-ass about modifications)

Quote
str_replace is still one of the fastest methods

In the uses given, sprintf is even faster.

Quote
I don't know what you are referring to regarding tabs and spaces. I use Komodo for files as my editor which is a proper code editor (has settings for each file type). It is set to 8 for the tabbing (4 or 8 is the norm & up to preference).. I can not see any issues regarding tabbing, to what are you referring?

The issue being referred to is that some lines, notably inside echos, have trailing spaces/tabs after the echo statement.

The example given:
echo '-->-->-->-->-->                               
--->-->--><a id="spamLink_' . $item['id']

There are multiple tabs in the first line of that which are still output by PHP, but shouldn't really be there. It's more of a code cleanliness thing not to mention a little bandwidth saving.
To assume is to hope that those who came before had the presence of mind and capacity to implement the dreams of those who would come after.

You either die a hero or live long enough to see yourself become the villain. It seems you have chosen which, and now I must do the same.

Offline Chen Zhen

  • Sophist Member
  • *****
  • Posts: 1,027
  • Gender: Male
  • If you're going through hell, keep going!
    • Underdog-01 on GitHub
    • WebDev.ca

  The actual html is tabbed in such a way to be lined up properly. Each echo drops down a line after a single quote to that effect and then is tabbed out to line up with the code that precedes it. If a tag is open ie. a div, span hyperlink, etc. , the code between the open & close of those tags is tabbed to show it is within it. When you peruse a file it all lines up properly as it should.

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 68,032
    • Arantor on GitHub
Um... there are extra spaces BEFORE the line break...
To assume is to hope that those who came before had the presence of mind and capacity to implement the dreams of those who would come after.

You either die a hero or live long enough to see yourself become the villain. It seems you have chosen which, and now I must do the same.

Offline Chen Zhen

  • Sophist Member
  • *****
  • Posts: 1,027
  • Gender: Male
  • If you're going through hell, keep going!
    • Underdog-01 on GitHub
    • WebDev.ca
  You respond faster than I can come back with something else.. geesh  :o .. If I understand you correctly, I can simply go through every file with Komodo and use the remove trailing whitespace command.

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 68,032
    • Arantor on GitHub
Yes, that's exactly what we're suggesting...
To assume is to hope that those who came before had the presence of mind and capacity to implement the dreams of those who would come after.

You either die a hero or live long enough to see yourself become the villain. It seems you have chosen which, and now I must do the same.